Implement pre-restart drain proof and hard gate #661
Closed
opened 2026-07-10 15:31:50 -05:00 by jcwalker3
·
1 comment
No Branch/Tag Specified
master
fix/issue-987-native-mcp-bootstrap
feat/issue-985-project-scoped-launcher-identity
fix/issue-983-cross-repo-base-ref
feat/issue-980-stale-worker-retirement
fix/issue-975-client-identity-heartbeat
fix/issue-973-cross-repo-canonical-roots
fix/issue-970-safely-resolve-missing-worktrees
fix/issue-969-native-mcp-bootstrap
feat/issue-664-break-glass-restart
feat/issue-708-mcp-namespace-attachment
feat/issue-665-restart-audit
fix/issue-700-durable-walls
fix/issue-704-prevent-env-workspace-bindings
feat/issue-707-cross-project-boundaries
fix/issue-690-review-profile-switch-guard
fix/issue-953-bootstrap-lock-provenance
feat/issue-949-native-fleet-inventory
fix/issue-943-runtime-context-helpers
fix/issue-945-owning-pr-renewal-evidence
fix/issue-941-scope-guard-bootstrap-wiring
docs/issue-930-remote-mcp-coupling-inventory
fix/issue-892-author-bootstrap-deadlock
fix/issue-686-detect-reject-manual-mcp
fix/issue-672-mcp-config-drift
fix/issue-689-deterministic-mcp-namespace
feat/issue-666-concurrent-mcp-restart-tests
feat/issue-659-maintenance-drain-mode
feat/issue-648-notifications-console
fix/issue-670-direct-master-incident
feat/issue-644-console-recovery
feat/issue-650-providers-insights
feat/issue-669-scoped-component-recovery
docs/issue-668-mcp-ha-rolling-restart
feat/issue-667-console-restart-controls
feat/issue-645-linkage-console
feat/issue-643-request-preview-initiate
fix/issue-897-permission-stale-runtime-classification
feat/issue-641-runtime-session-view
feat/issue-663-restart-classes
feat/issue-661-drain-proof-hard-gate
fix/issue-854-semantic-container-exclusion
issue-640
fix/issue-682-starlette-httpx2
v1.1.0
Labels
Clear labels
allocator
anti-stomp
architecture
bug
chore
codex
concurrency
contamination
control-plane
dashboard
database
design
documentation
enhancement
gitea
glitchtip
important
incident
incident-bridge
integration
jenkins
labels
leases
mcp
mcp-health
mcp-menu
multi-project
mutating
nice-to-have
observability
portability
preflight
protected-branch
queue
read-only
reconnect
recovery
refactor
release
reliability
resumable-review
reviewer
roadmap
safety
security
self-hosted
sentry
stale-runtime
status:blocked
status:in-progress
status:pr-open
status:ready
terminal-lock
testing
tracker
type:bug
type:feature
type:feature
type:guardrail
visibility
workflow
workflow-hardening
workflow-hardening
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Controller-owned work allocator
Prevent concurrent LLM session stomping
Architecture / structural design
OpenAI Codex client / workflow session surface
Concurrent session safety
Workflow or session contamination incident
MCP control-plane coordination and allocation authority
MCP operational dashboard/queue view
Internal coordination storage (SQLite/Postgres)
Design / investigation, no implementation
Docs / runbooks
New feature or improvement
Gitea MCP workflow
GlitchTip integration
Operational or process incident requiring durable audit trail
Sentry-to-Gitea incident bridging
Integration testing
Jenkins integration
Label taxonomy management
Lease adopt/release/expire lifecycle
MCP server / tooling
MCP namespace and runtime health
MCP menu surface
Work spanning multiple monitoring projects or Gitea repos
Mutating action; requires gating
Observability, metrics, traces, error reporting
Cross-platform / portability
Shared preflight gates before mutation
Protected branch / stable-branch policy concern
Work queue visibility and allocation
Read-only, no mutation
MCP client reconnect/reload recovery path
Recovery paths for stale/foreign leases
Code refactor / restructure
Release / versioning
Reliability / failure handling
Persist and resume prepared review verdicts across sessions
Reviewer workflow tooling
Roadmap / umbrella issue
Safety rails and fail-closed mutation guards
Security / trust boundary
Self-hosted infrastructure integration
Sentry error monitoring integration
Stale backend daemon / runtime-vs-master parity failures
Issue is blocked
Issue is being worked on
Issue has an open pull request
Issue is ready for work
Terminal review lock (#332) path
Tests / test coverage
Issue tracker hygiene / meta
Bug or defect
Feature or enhancement
Feature or enhancement
Safety gate or guardrail
Workflow state visibility for LLMs/operators
Cross-tool workflow
LLM workflow coordination hardening
LLM workflow coordination hardening
Something is not working
This issue or pull request already exists
New feature
Need some help
Something is wrong
More information is needed
This won't be fixed
Milestone
No items
No Milestone
Projects
Clear projects
No projects
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Scaled-Tech-Consulting/Gitea-Tools#661
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Problem statement
Even with drain mode, restart can proceed without a machine-verifiable proof that unsafe work is clear and checkpoints complete.
Operational impact
False “ready to restart” claims; data loss; dogfooding fails.
Scope
Non-goals
Required implementation investigation
#655 #658 #659 #660; final_report style validators; crypto/hash for proof id.
Proposed implementation direction
DrainProofschema +verify_drain_proof()fail closed; wire into restart apply.Security / workflow-safety
Proof non-forgeable within process; short TTL; audit.
Failure behavior
Missing/stale/failed proof → deny restart; create incident work item.
Acceptance criteria
Required tests
Gate unit tests; multi-session unsafe mutation case.
Observability and audit
Proof id in restart audit events.
Dependencies
Rollout
Enforce in dogfood after dry-run period.
Canonical issue state
Required proof
Code + tests + sample proof JSON.
Required final response and handoff
Brief PR → reviewer.
[THREAD STATE LEDGER]
What is true now
576(APPROVE, head1cbbde00895dce1545290a660bc31c4ac9210b3e, not dismissed) is still the live server-side verdict, andapproval_at_current_headis still true for that head.1cbbde00895dce1545290a660bc31c4ac9210b3eand could not record it; it exists only in this comment.1cbbde00895dce1545290a660bc31c4ac9210b3e(live head re-pinned before review; unchanged).cccecorrected:ccde9e8f1183bc9a6672393a9923f304086fb874.What changed
git status --porcelainempty and HEAD confirmed before and after).What is blocked
The terminal REQUEST_CHANGES cannot be recorded on PR #882.
gitea_mark_final_review_decision(pr_number=882, action=request_changes, expected_head_sha=1cbbde00895dce1545290a660bc31c4ac9210b3e)returnedmarked_ready: falseon both attempts withclassification: foreign_pr_terminal, citing a terminal approve on PR #885 (review580, headdac40ab9b3ef13255c4fe0f6353e6d93e0d0d885).gitea_diagnose_review_decision_lock(target_pr_number=882)simultaneously returnedmark_final_allowed: true,recovery_allowed: true,recovery_mode: cross_pr_isolation, and the reason"last terminal is approve on foreign open/closed PR #885; target PR #882 is isolated (#693)".Both tools prescribe retrying the identical call, so the prescribed recovery is a loop. It was attempted exactly twice and then stopped.
final_review_decision_readywas not forced andgitea_authorize_review_correctionwas not misused as a cross-PR unlock. Every PR review path is gated behind this same flag — the non-terminal COMMENT path also refused, with"final_review_decision_ready must be true; validation-phase live review mutations are forbidden"— so this issue thread is the only channel available to the reviewer role.Safety impact: because no terminal verdict could be recorded, PR #882 remains merge-eligible under approval
576despite a reproduced fail-open. Treat this comment as a hard block on merging PR #882.Who/what acts next
drain_proof.pyso absent, malformed, stale, or unproven acknowledgement data fails closed; add the absent-key regression test; push a new head onfeat/issue-661-drain-proof-hard-gate. The new head will invalidate approval576and force a fresh review, which also clears this deadlock.1cbbde00895dce1545290a660bc31c4ac9210b3e; do not treat review576as a current reviewer verdict; do not delete session-state files; do not callgitea_authorize_review_correctionas a cross-PR unlock; do not rebase or force-push the branch.Blocker A1 — acknowledgement evaluation fails open when acknowledgement data is absent
drain_proof.py,_evaluate_checks, check 6 (acks_or_timeout):When the acknowledgement key is absent,
drain_state.get("acks") or {}collapses to{}. The empty mapping satisfiesno_sessions_to_ack, soacks_okbecomesTrueand the check reports"no other live sessions required to acknowledge".That conclusion is reached without consulting the impact report. The report's
counts.sessions_live_otheris never read by this check, so "no acknowledgement data exists" and "it was proven that nobody needed to acknowledge" collapse into a single state. The first is absence of evidence; the second is evidence of absence.This is the only checklist item in the file that does not fail closed. Check 1 refuses on
inventory_complete=False; checks 2, 3 and 4 refuse via_bool_inputon unknown values; check 5 refuses when lease disposition is unasserted. Check 6 is the exception, and it is the item gating whether live sessions are interrupted by a restart.Reproduction at the exact head
Impact report asserts three other live sessions. Drain state asserts every other outcome true and omits both
acksandack_timeout_policy_applied.A restart that would interrupt three live sessions is authorized by a proof carrying zero acknowledgement evidence. Because the proof mints
clean, it also passesverify_drain_proofandgate_apply_restartdownstream with a valid signature: the gate does not merely fail to notice, it mints a positively-signed clean artifact attesting to a drain that was never demonstrated.impact_fingerprint()does covercounts.sessions_live_other, but binding only detects a changed report. Nothing compares the live-session count against the acknowledgement evidence, so the contradiction between "3 live sessions" and "no acknowledgements" is never evaluated.Required invariant
Missing, malformed, stale, or unproven acknowledgement data must fail closed; only explicitly verified acknowledgement data may permit the operation.
acks_or_timeoutmust pass only when one of the following is affirmatively demonstrated:counts.sessions_live_other == 0), rather than inferring it from an empty or absent mapping; orAn absent key, a non-mapping value, a mapping whose keys do not cover the live sessions named by the report, or any unparseable acknowledgement value must all produce
passed=False.Blocker A2 — no test covers the absent-key path
tests/test_drain_proof.pyhas 25 cases and exercises each failing checklist item, but every acknowledgement case supplies anacksmapping or sets the timeout policy. No case omits the key while the report shows live sessions, which is why A1 passed a green suite and two prior reviews. A regression test assertingpassed=Falseandproof.clean=Falseforsessions_live_other > 0with the acknowledgement key absent is required alongside the fix.Not in dispute
The remainder of the change is sound and needs no rework: the keyed HMAC-SHA256 proof id over a canonical body, the per-process secret defeating cross-restart replay, the 120s TTL, recomputing cleanliness rather than trusting a presented
cleanflag, deriving checks 1 and 5 from the impact report instead of caller self-report, and the non-silent break-glass bypass are all correctly built. A1 is a single inverted default inside an otherwise fail-closed design.Canonical Issue State
STATE:
changes-requested
WHO_IS_NEXT:
author
NEXT_ACTION:
Fix the acks_or_timeout fail-open in drain_proof.py so absent, malformed, stale, or unproven acknowledgement data fails closed, add the absent-key regression test, and push a new head on feat/issue-661-drain-proof-hard-gate.
NEXT_PROMPT:
WHAT_HAPPENED:
Reviewer re-reviewed PR #882 at head
1cbbde0089and reproduced an acknowledgement fail-open: with counts.sessions_live_other = 3 and the acknowledgement key absent, acks_or_timeout passed, the proof minted clean, and gate_apply_restart returned verdict allow. The terminal REQUEST_CHANGES could not be recorded because the review-decision lock refused twice with classification foreign_pr_terminal citing PR #885.WHY:
drain_proof.py check 6 treats an absent acknowledgement mapping as proof that no acknowledgement was required, never consulting counts.sessions_live_other, making it the only checklist item in the file that does not fail closed.
RELATED_PRS:
#882 (this review, head
1cbbde0089); #885 (holder of the foreign terminal decision that refused the lock, review 580 at headdac40ab9b3)BLOCKERS:
A1 acknowledgement fail-open in drain_proof.py acks_or_timeout, reproduced at the exact head. A2 no regression test covers the absent-key path. Blocker classification for this thread: process/rule blocker — the terminal reviewer verdict could not be recorded because of the cross-PR review-decision lock described above.
VALIDATION:
Executable reproduction against an unmodified worktree pinned to
1cbbde0089, HEAD and clean status confirmed before and after: sessions_live_other=3, acknowledgement key absent, acks_or_timeout.passed=True, detail "no other live sessions required to acknowledge", proof.clean=True, failed_checks=[], gate verdict allow, allow=True. No test suite was re-run for this finding; the defect is demonstrated directly against the shipped module at the pinned head.LAST_UPDATED_BY:
sysadmin / prgs-reviewer / 2026-07-25