Security review: surface the session's reason when it deliberately ends incomplete #607

Activity

1. JacobStephens2 commented on Oct 10, 2026

   

   JacobStephens2

   on Oct 10, 2026

   OwnerAuthor

   More actions

   This was generated by AI during triage.

   Agent Brief

   Category: bug
   Summary: Give the Security review an explicit incomplete final line, and surface its reason in the Run's Cause

   Current behavior:
   The Security review prompt defines a single terminal line, Security review: {"unaddressed_count":…,"findings":[…],"pre_existing_count":…}, which is required only after a complete review. When the session deliberately ends incomplete (for example, no sandbox, a missing prerequisite or a blocked validation), it ends with prose. Review::from_final_message fails, and security::review::Outcome::of maps every error except SafeguardRefusal to the same fixed text: "Security review incomplete: session failed, ended early or omitted a valid final line; see Session log". So a well-explained, deliberate incomplete looks the same as a crash.

   Desired behavior:

   • The Security review prompt defines a second terminal line: Security review: incomplete: <one-line reason>. The prompt asks for it whenever the review is incomplete or missing a prerequisite, and keeps the JSON line for complete reviews. The audit's Security audit: incomplete protocol is the model for this.
   • Parsing the final message tells three cases apart:
     1. a valid JSON line, which is a complete review (unchanged);
     2. an explicit incomplete line with a nonempty, single-line reason, which becomes an incomplete outcome carrying that reason;
     3. anything else (no message, no recognizable line, invalid JSON, inconsistent counts), which keeps today's generic text.
   • In case 2, the Run's Cause line, the PR's "Security review outcome" entry and the Hold all show the session's reason. Example: Security review incomplete: required sandbox isolation is unavailable.
   • The private-report check still applies to explicit incompletes: if the report file holds entries, they are still validated and recorded, or the mismatch is reported. Pre-existing vulnerability details must never reach the public reason. The reason comes from the session, so treat it as public text under the same rule the prompt already applies to the final message, and cap its length.
   • Safeguard refusals keep their existing "Security review refused: …" wording.

   Key interfaces:

   • prompt::security_review: the final-line contract text. Regenerate prompts/ with UPDATE_PROMPTS=1 cargo test prompts_page; don't edit prompts/ by hand.
   • Review::from_final_message: today it returns Result<Review>. It needs a way to return "incomplete, with reason" that is different from a parse failure, for example a dedicated error type that Outcome::of downcasts as it does SafeguardRefusal, or an enum return.
   • security::review::Outcome::of / Outcome::Incomplete(String): carries the session's reason.

   Acceptance criteria:

   • A final message ending Security review: incomplete: required sandbox isolation is unavailable produces an incomplete outcome whose Cause and PR entry include that reason.
   • A final message with no recognizable final line, invalid JSON or inconsistent counts still produces the existing generic incomplete text.
   • A valid JSON final line behaves exactly as it does today.
   • An empty or multi-line incomplete reason is treated as an invalid final line (generic text).
   • The safeguard-refusal wording is unchanged.
   • The generated prompts/ matches the updated prompt module.

   Out of scope:

   • Making a sandbox available, or checking for one in thirdshift setup (tracked separately).
   • Changing whether an incomplete Security review holds the Run.
   • Changes to the Security audit's terminal protocol.
2. JacobStephens2

   added

   bugSomething isn't workingSomething isn't working

   ready-for-agentFully specified, ready for an AFK agentFully specified, ready for an AFK agent

   in-progressA Claim: thirdshift has taken this issueA Claim: thirdshift has taken this issue

   and removed

   ready-for-agentFully specified, ready for an AFK agentFully specified, ready for an AFK agent

   in-progressA Claim: thirdshift has taken this issueA Claim: thirdshift has taken this issue

   on Oct 10, 2026
3. JacobStephens2

   closed this as completedin #612on Oct 10, 2026
4. JacobStephens2

   removed

   in-progressA Claim: thirdshift has taken this issueA Claim: thirdshift has taken this issue

   on Oct 10, 2026