You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
F: a draft names the budget, not an unsaved safety finding from preparation #127
A draft PR can name the wrong reason when a task's safety finding was never saved.
A run that finds a safety violation saves the finding on its attempt, then moves the task to needs human. If the save fails (a real SQLite error) and the move succeeds, the attempt row has no safetyFinding. The finding text exists only in memory, and SafetyFindings.settle drops it once the move commits.
TaskPublishing.#problems (runner/publishing.ts:330) then picks the draft's problems in this order:
A finding remembered in #escalations. Only #payOwed fills this, so a run's own escalation is never in it.
The current attempt's safetyFinding. This is empty, because the save failed.
A diagnostic that starts with Safety violation: (SAFETY_VIOLATION).
"The task's time budget ran out…" when the budget has passed.
The attempt's diagnostic.
Step 3 covers findings from the audit, whose diagnostic is the finding text. It misses two other sources:
A finding raised during preparation. Its diagnostic is Preparation failed: "Safety violation: …", which does not start with the prefix.
A finding from the audit of a failed run. Its diagnostic is the agent's quoted stderr.
Failure scenario
A task's budget has passed.
A run raises a safety finding during preparation, and recordSafetyFinding fails.
The run still moves the task to needs human.
The draft PR says the time budget ran out, and does not mention the safety finding.
The contract in docs/implementation/pull-request-opening.md (Mode) says a budget that ran out must not hide a finding.
How likely
Very unlikely today. The save goes through the shutdown capability, so only a storage failure triggers it, and it also needs one of the two finding sources above. The independent review of PR #110 (round 10) found it and rated it below the bar for that PR.
Proposed fix
Make ItemExecutor remember the reason for every escalation it makes, keyed by task and tied to the attempt that is current after the move. That covers a run's own #escalate as well as payOwed. Then have TaskPublishing.#problems read that, instead of only its own #escalations map and the SAFETY_VIOLATION prefix check.
Use the same rule as now: the remembered reason applies only while the task is still in needs human on that attempt (see the round 8 fix in #110).
Done when
A draft for each finding source names the finding, even after the budget has passed: audit, preparation, and the audit of a failed run.
A test for each source fails when the remembered reason is removed.
pull-request-opening.md describes where the reason comes from.
Activity