Skip to content

Post a failed Run's Recovery, and a waiting Spec's reasons, as a generated issue comment - #1940

Merged
JacobStephens2 merged 4 commits into
issue-1901from
issue-1904
Nov 10, 2026
Merged

JacobStephens2 merged 4 commits into
issue-1901from
issue-1904

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Nov 10, 2026 •

Copy link
Copy Markdown
Owner

Summary

A Run that ends agent-failed now posts its Recovery on the issue as one generated comment. A Spec run that ends only waiting posts one comment on its Spec, naming each undone Ticket and what it waits on. The Claim ending decides whether a comment is owed, so the posting rule sits next to the label rule:

 Claimed::end(ending) -> owes a comment
   NotReproduced | Stop                 -> false
   Waiting        -> hand_back          -> true
   Failed, Security fix -> label_failed -> false   # stays in the private advisory
   Failed         -> released           -> false   # a retry loop can't fill the issue
   Failed         -> label_failed       -> true    # labelled agent-failed, or tried to
   everything else                      -> false
+Claim::end(ending, comment)
+  if owed: post comment; a failed post is a warning, and the outcome is unchanged
This comment was generated by AI.

thirdshift's Run on this issue failed, so it waits for the Day shift.

- Cause: claude exited 3
- Stopped at: Implementation
- Preserved: Issue branch `issue-7` on origin, with pull request https://github.com/acme/widgets/pull/12
- Machine: night-host

Put `ready-for-agent` back to retry.
  • src/recovery.rs: renders the comment from factory-owned facts only, with no continue command. Local paths in the cause become <local path>. If the Spec PR changed under the Spec run, the retry line says to decide the Base branch and Spec branch first. A Run with no Recovery says Stopped at: Start, before any work began.
  • src/run.rs and src/processes.rs: Ticket runs, Spec runs and Custom processes pass the comment to the Claim's ending. A Custom process that fails before its Steps names its Issue branch, and the pull request when it is a Continuation with one open.
  • src/claim.rs: before posting, the Claim checks the comment against this repository's stored private Security review reports. It captures them before the run's sessions and again at the end. If the comment includes any of their text, a withheld comment is posted in its place. A failed session's diagnostic is model output, and it appears in the cause.
  • src/github.rs: the AI-attribution line is now one constant, GENERATED, which Triage runs use as well.
  • docs/guide.md (regenerated into site/docs/index.html) and the hand-written site/index.html, site/how-it-works/, site/architecture/ describe the comment.

Closes #1904

Evidence

Execution evidence

  • Before: this branch's tests run against the base branch's src/ (git checkout origin/issue-1901 -- src, then cargo test --test failed_run a_failed_run_left_agent_failed_posts):
    test a_failed_run_left_agent_failed_posts_its_recovery_comment_and_again_when_it_fails_again ... FAILED
    panicked at tests/failed_run.rs:507:5:
      left: 0
     right: 1
    test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 17 filtered out
    
    After: cargo test --test failed_run:
    test a_failed_run_left_agent_failed_posts_its_recovery_comment_and_again_when_it_fails_again ... ok
    test a_recovery_comment_that_cannot_be_posted_is_a_warning_and_the_run_still_fails_agent_failed ... ok
    test an_interrupted_run_that_left_nothing_on_origin_releases_its_claim ... ok
    test result: ok. 17 passed; 0 failed; 1 ignored
    
  • cargo test --test spec_run -- waiting failed_ticket blocking_cycle covers the waiting Spec comment and the failed Spec run's Recovery comment: ok. 15 passed.
  • cargo test --test process covers the failed Custom process comment: ok. 237 passed; 1 ignored.
  • cargo test --test security_run the_security_run_ends_as_a_failed_fix checks that a failed Security fix posts no comment: ok. 1 passed.
  • npm run docs:check --prefix site passes.
  • Security review: cargo test --test failed_run a_recovery_comment_never_posts_private_security_report_material_its_cause_quoted failed on this branch before the fix: the comment's cause quoted a planted private report line. It passes after the fix. Afterwards cargo test --test failed_run gave ok. 18 passed; 1 ignored and cargo test --test process gave ok. 237 passed; 1 ignored. scripts/affected-tests main was run, but rustc was OOM-killed (SIGKILL) while compiling the unit-test binary, the same host limit described below. cargo clippy --all-targets typechecks it.
  • Not run locally: the unit-test binary (cargo test --bin thirdshift) never finished compiling. rustc was OOM-killed on every attempt, including -j 1, CARGO_INCREMENTAL=0 and debuginfo off, with about 2 GB free on a host shared with other sessions. So the new inline tests in src/claim.rs and src/recovery.rs have only been typechecked (cargo check --tests, clean); CI will be their first run. That includes the three tests taken from the Spec review. scripts/affected-tests was not run, for the same reason.

Visual evidence

None. The site changes only add sentences inside existing paragraphs and list items, and no screenshots were taken.

Merge Danger

Door: two-way

Posted comments stay on issues after a revert, but reverting stops new ones.

Blast Radius: issue-comments

Every agent-failed failure, except a failed Security fix, now writes a public comment as the operator's gh account. A cause with a local path in an unusual form, such as one containing spaces or a relative path, could still slip past the scrubbing.

Unaddressed findings

Standards

  • 3. Data Clumps in recovery::comment's five parameters. Declined. machine is a parameter so the tests stay deterministic, which the codebase-design skill endorses ("Accept dependencies"). The free function is the single entry point for both the Recovery and no-Recovery cases (src/recovery.rs, pub fn comment).
  • 4. The comment is built eagerly and the owed-or-not decision is a bool. Partly addressed: the hard-to-parse doc comment on Claim::end is rewritten. Moving the comment into Ending is declined. Ending is Copy and compared by value in the existing Claim, Run and process tests (src/run.rs Call::EndClaim(claim::Ending)). The rule for whether a comment is owed already lives in one place, Claimed::end in src/claim.rs. Building a string that is never posted costs nothing that matters.
  • 7. Feature Envy: TicketsNotDone::comment lives in src/spec_run.rs. Declined. It renders the not_done field private to TicketsNotDone. Moving it to recovery.rs would expose that field for no gain. The two comments share only the attribution line, which is now crate::github::GENERATED.

Security

None. The one introduced finding, private Security review material reaching the public Recovery comment through a failed session's diagnostic, is fixed and covered by a regression test.

Spec

  • 1, the pull-request part. Partly addressed: the stopping point is now always present (Stopped at: Start, before any work began), and a Custom process that fails before its Steps on a Continuation names that Continuation's open pull request (before_steps_comment in src/processes.rs). The remaining case is a Run with no Recovery and no recorded pr_url. Its comment names the Issue branch but no pull request, because none was observed and looking one up would add a GitHub call to a failure path.

Every changed file was read by both reviewers except site/docs/index.html. It is generated from docs/guide.md, and npm run docs:check --prefix site passed. The Standards reviewer inspected its diff, and the Spec reviewer excluded it as generated output.

Built with claude 2.1.314 · claude-opus-5-5 · medium

Security review outcome

No unaddressed introduced findings.

…ec PR caveat, scrub joined paths, share the attribution line
A failed session's diagnostic is model output and goes into the Recovery
comment's cause. Check the comment against the stored private Security
reports, captured before the run's sessions and again at its end, and post
a withheld comment in its place when it includes any of their text.
…ment's pull request

Clippy's wildcard_enum_match_arm lint rejects the catch-all arm.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant