NightProwl: audit release-engine claim and handoff race boundaries #788

New Issue

rockachopa · 2026-08-14T01:02:35Z

rockachopa commented

2026-08-14 01:02:35 +00:00

Read-only audit of the release-engine claim, lease, handoff, and review path. Identify at most five concrete race or partial-failure risks with exact source paths and deterministic tests. Distinguish currently enforced invariants from documented expectations. Do not edit or post speculative vulnerabilities without source evidence.

rockachopa added the

agent:nightprowl

label 2026-08-14 01:02:35 +00:00

nightprowl was assigned by rockachopa

2026-08-14 01:02:35 +00:00

nightprowl commented

2026-08-14 02:44:50 +00:00

[NIGHTPROWL EVAL]

NIGHTPROWL_FINAL: AUDIT: stackchain-dashboard release-engine (sprint-577, src/release_engine.py; docs/release-engine-spec.md). Read-only analysis of claim/lease/handoff/review paths. Five concrete risks with exact paths, deterministic tests, and invariant classification. --- RISK 1 — Claim PATCH has no rollback on verification failure (PARTIAL-FAILURE) Path: src/release_engine.py:418-423. claim_issue PATCHes Gitea, then GETs and verifies assignee. If verification fails (e.g., external reassignment between PATCH and GET), RuntimeError is raised but the issue remains PATCHed to the agent — no compensating PATCH to unassign. Result: orphan claim, issue stuck assigned to agent with no progress and no checkpoint. Enforced: claim PATCH + GET verification exists. Documented gap: spec acceptance test 3 ("Claim must be verified from Gitea before execution") does not cover verification-failure rollback. Deterministic test: mock GiteaClient.claim_issue succeeds, get_issue returns assignee="other_agent"; assert issue remains assigned to agent after RuntimeError (no unassign PATCH called); assert no state saved. RISK 2 — Crash between claim PATCH and state save leaves orphan claim with no resume path (PARTIAL-FAILURE) Path: src/release_engine.py:418-424. If process dies after PATCH but before store.save(RunState(..., "claimed")), the issue is assigned to the agent but no checkpoint exists. On restart, select_issue sees assignee==agent (eligible), re-runs claim_issue (idempotent PATCH), re-executes from scratch. Issue stays claimed the entire crash window with no progress recorded. Worse: if agent_timeout_seconds is long and crash is silent, issue is locked to a dead agent. Enforced: state saved after claim. Documented gap: spec says "Claim must be verified before execution" but does not address crash orphan. Deterministic test: mock client where claim_issue succeeds, then raise OSError before store.save; assert issue remains assigned; assert on restart select_issue returns same issue and re-claims. RISK 3 — Checkpoint resume bypasses claim verification (ENFORCED-INVARIANT vs DOCUMENTED-EXPECTATION MISMATCH) Path: src/release_engine.py:387-413. On resume from agent_complete/tests_passed/pushed, code checks out branch, verifies HEAD sha and clean worktree — but does NOT re-verify the Gitea assignee. Spec acceptance test 3 says "Claim must be verified from Gitea before execution." Resume path allows execution to proceed even if an external actor reassigned the issue after the initial claim. Enforced: resume checks branch/sha/worktree. Documented gap: spec does not distinguish initial-claim verification from resume-time verification. Deterministic test: create checkpoint with status="agent_complete"; mock get_issue to return assignee="reassigned_other"; resume execute(); assert it proceeds to tests without raising (demonstrating the bypass). To fix, resume should re-verify assignee matches agent. RISK 4 — TOCTOU race between find_open_pr and create_pr (RACE) Path: src/release_engine.py:354-362 (existing PR check) vs later PR creation (line 501+, truncated). Two concurrent invocations (different repo_path or external process) can both see no existing PR and both create one for the same head branch. RepositoryLease only locks same repo_path; cross-path or external PRs are unprotected. Enforced: same-repo_path runs are serialized. Documented gap: spec says "Existing open PR for the same head branch is reused" but check-then-act is not atomic. Deterministic test: mock find_open_pr returns None; simulate concurrent PR creation between check and create_pr; assert two PRs created for same branch. Mitigate with Gitea-side idempotency or pre-push PR check. RISK 5 — Lease held through entire execution including long agent/test runs (RACE BOUNDARY / AVAILABILITY) Path: src/release_engine.py:336-337. RepositoryLease is acquired before Gitea discovery and held through agent run (default 1800s) and test run (default 900s). A single slow/hung run blocks all other invocations on the same repo_path for up to ~45 minutes. Combined with Risk 2 orphan claims, this creates a wide window where no other worker can make progress. Enforced: single-writer lease per repo_path. Documented gap: spec says "A contended invocation fails immediately" but does not bound lease duration or address hung-run impact. Deterministic test: mock ShellRunner.run that blocks beyond timeout_seconds; assert second execute() raises "release run already active" for duration of first run. Mitigate with watchdog or lease TTL. --- ACCEPTANCE CRITERIA for fixes: - Risk 1: On claim-verification failure, PATCH assignee back to previous value (or None) before raising; test asserts unassign PATCH called. - Risk 2: Write checkpoint BEFORE claim_issue (claim becomes resumable); or add orphan-recovery sweeper; test asserts checkpoint exists for crash-after-claim. - Risk 3: Resume path re-verifies get_issue.assignee == agent; test asserts resume raises on reassigned issue. - Risk 4: Atomic PR creation (e.g., rely on Gitea 409 on duplicate, or check-after-create); test asserts exactly one PR for concurrent attempts. - Risk 5: Lease acquisition after agent/test, or lease TTL with renewal; test asserts second invocation acquires after bounded wait. UNCERTAINTY: PR creation logic (lines 501+, truncated) not fully reviewed — duplicate-PR risk depends on whether create_pr has server-side dedup. Gitea claim PATCH idempotency assumed but not verified against real Gitea behavior. No test file was read in full — listed tests are inferred from spec acceptance criteria; actual test coverage gaps may differ. EVIDENCE: /root/stackchain-dashboard-sprint-577/src/release_engine.py (lines 336-337 lease; 354-362 PR check; 387-413 resume; 418-424 claim; 457-481 test; 483-491 push); /root/stackchain-dashboard-sprint-577/docs/release-engine-spec.md (state machine; acceptance tests 1-17).

[NIGHTPROWL EVAL] NIGHTPROWL_FINAL: AUDIT: stackchain-dashboard release-engine (sprint-577, src/release_engine.py; docs/release-engine-spec.md). Read-only analysis of claim/lease/handoff/review paths. Five concrete risks with exact paths, deterministic tests, and invariant classification. --- RISK 1 — Claim PATCH has no rollback on verification failure (PARTIAL-FAILURE) Path: src/release_engine.py:418-423. claim_issue PATCHes Gitea, then GETs and verifies assignee. If verification fails (e.g., external reassignment between PATCH and GET), RuntimeError is raised but the issue remains PATCHed to the agent — no compensating PATCH to unassign. Result: orphan claim, issue stuck assigned to agent with no progress and no checkpoint. Enforced: claim PATCH + GET verification exists. Documented gap: spec acceptance test 3 ("Claim must be verified from Gitea before execution") does not cover verification-failure rollback. Deterministic test: mock GiteaClient.claim_issue succeeds, get_issue returns assignee="other_agent"; assert issue remains assigned to agent after RuntimeError (no unassign PATCH called); assert no state saved. RISK 2 — Crash between claim PATCH and state save leaves orphan claim with no resume path (PARTIAL-FAILURE) Path: src/release_engine.py:418-424. If process dies after PATCH but before store.save(RunState(..., "claimed")), the issue is assigned to the agent but no checkpoint exists. On restart, select_issue sees assignee==agent (eligible), re-runs claim_issue (idempotent PATCH), re-executes from scratch. Issue stays claimed the entire crash window with no progress recorded. Worse: if agent_timeout_seconds is long and crash is silent, issue is locked to a dead agent. Enforced: state saved after claim. Documented gap: spec says "Claim must be verified before execution" but does not address crash orphan. Deterministic test: mock client where claim_issue succeeds, then raise OSError before store.save; assert issue remains assigned; assert on restart select_issue returns same issue and re-claims. RISK 3 — Checkpoint resume bypasses claim verification (ENFORCED-INVARIANT vs DOCUMENTED-EXPECTATION MISMATCH) Path: src/release_engine.py:387-413. On resume from agent_complete/tests_passed/pushed, code checks out branch, verifies HEAD sha and clean worktree — but does NOT re-verify the Gitea assignee. Spec acceptance test 3 says "Claim must be verified from Gitea before execution." Resume path allows execution to proceed even if an external actor reassigned the issue after the initial claim. Enforced: resume checks branch/sha/worktree. Documented gap: spec does not distinguish initial-claim verification from resume-time verification. Deterministic test: create checkpoint with status="agent_complete"; mock get_issue to return assignee="reassigned_other"; resume execute(); assert it proceeds to tests without raising (demonstrating the bypass). To fix, resume should re-verify assignee matches agent. RISK 4 — TOCTOU race between find_open_pr and create_pr (RACE) Path: src/release_engine.py:354-362 (existing PR check) vs later PR creation (line 501+, truncated). Two concurrent invocations (different repo_path or external process) can both see no existing PR and both create one for the same head branch. RepositoryLease only locks same repo_path; cross-path or external PRs are unprotected. Enforced: same-repo_path runs are serialized. Documented gap: spec says "Existing open PR for the same head branch is reused" but check-then-act is not atomic. Deterministic test: mock find_open_pr returns None; simulate concurrent PR creation between check and create_pr; assert two PRs created for same branch. Mitigate with Gitea-side idempotency or pre-push PR check. RISK 5 — Lease held through entire execution including long agent/test runs (RACE BOUNDARY / AVAILABILITY) Path: src/release_engine.py:336-337. RepositoryLease is acquired before Gitea discovery and held through agent run (default 1800s) and test run (default 900s). A single slow/hung run blocks all other invocations on the same repo_path for up to ~45 minutes. Combined with Risk 2 orphan claims, this creates a wide window where no other worker can make progress. Enforced: single-writer lease per repo_path. Documented gap: spec says "A contended invocation fails immediately" but does not bound lease duration or address hung-run impact. Deterministic test: mock ShellRunner.run that blocks beyond timeout_seconds; assert second execute() raises "release run already active" for duration of first run. Mitigate with watchdog or lease TTL. --- ACCEPTANCE CRITERIA for fixes: - Risk 1: On claim-verification failure, PATCH assignee back to previous value (or None) before raising; test asserts unassign PATCH called. - Risk 2: Write checkpoint BEFORE claim_issue (claim becomes resumable); or add orphan-recovery sweeper; test asserts checkpoint exists for crash-after-claim. - Risk 3: Resume path re-verifies get_issue.assignee == agent; test asserts resume raises on reassigned issue. - Risk 4: Atomic PR creation (e.g., rely on Gitea 409 on duplicate, or check-after-create); test asserts exactly one PR for concurrent attempts. - Risk 5: Lease acquisition after agent/test, or lease TTL with renewal; test asserts second invocation acquires after bounded wait. UNCERTAINTY: PR creation logic (lines 501+, truncated) not fully reviewed — duplicate-PR risk depends on whether create_pr has server-side dedup. Gitea claim PATCH idempotency assumed but not verified against real Gitea behavior. No test file was read in full — listed tests are inferred from spec acceptance criteria; actual test coverage gaps may differ. EVIDENCE: /root/stackchain-dashboard-sprint-577/src/release_engine.py (lines 336-337 lease; 354-362 PR check; 387-413 resume; 418-424 claim; 457-481 test; 483-491 push); /root/stackchain-dashboard-sprint-577/docs/release-engine-spec.md (state machine; acceptance tests 1-17).

rockachopa commented

2026-08-14 09:12:51 +00:00

⏱ Escalation: this ticket has been assigned to nightprowl for 8.2h with no progress.