main · last commit
13 days ago ·
7g0stsfu
ah-1cx.9 [bug] failure comment renders 'exit code 0' and drops the provider's rejection text
Past Stand
bd reopen ah-1cx.9
| Created by | Eugene Blikh |
| Owner | bigbes@gmail.com |
| Created | 2026-08-04T23:42:47Z |
| Started | 2026-08-04T23:54:11Z |
| Updated | 2026-08-05T00:22:44Z |
| Closed | 2026-08-05T00:22:44Z |
Found by the independent review of 665e805 (ah-tqc). Since ah-tqc, an error outcome can carry exit code 0. failureComment (internal/reconcile/comments.go:209-214) prints the exit code whenever outcome == domain.OutcomeError, and its own doc comment says 'exit code (only for an error outcome, when the process actually produced one)' โ an invariant ah-tqc silently broke. For the exact live incident ah-tqc exists to fix, the operator now sees on the card: ๐ค attempt 1 failed ยท outcome `error` ยท exit code 0 with no diff stat (there is none) and nothing else. Meanwhile info.FinalErrorMessage โ '401 {"type":"error","error":{"type":"ModelError","message":"Model claude-3-5-haiku is not supported"}}', the one string that makes this diagnosable in five seconds instead of an hour โ goes only to the daemon's log.Warn in runner.go finalizeFromExit. The card is the operator-facing surface and the whole reason the bug mattered; 'error, exit code 0' reads like a daemon bug rather than a broken model config.
Two changes, one of them cross-package. 1. Gate the exit-code clause in failureComment on exitCode != 0, restoring the invariant its doc comment states. Cheap, local to internal/reconcile/comments.go. 2. Thread FinalErrorMessage to the card. It is parsed in internal/runner (EventStreamInfo.FinalErrorMessage) and needs to reach failureComment, so it crosses ports.RunStatus โ domain.Run โ the comment. Decide deliberately whether it should also be PERSISTED on the run row (useful for /api/v1/status and for a post-hoc audit) or merely passed through to the comment. Persisting means a store migration; passing through means the information is lost on a refinalize, which reads the stored row rather than st. Truncate it โ a provider can return a large body, and SPEC's comment-size discipline applies. Do this AFTER the ah-tqc follow-ups land in internal/runner, since they touch the same struct.
A run finalized as error with exit code 0 produces a card comment that names the provider's rejection and does NOT claim an exit code. A run with a genuine non-zero exit still shows it. Covered by a reconcile test asserting both shapes.
ah-1cx
โ Hardening: post-audit bug sweep (2026-07-20)
parent-child
open
Nothing depends on this issue.
| id | ah-1cx.9 |
| content_hash | 3cd7bb97a8755572aeb1bbbdf902779271b259e483d881fd0040b5a780c3c811 |
| title | [bug] failure comment renders 'exit code 0' and drops the provider's rejection text |
| description | Found by the independent review of 665e805 (ah-tqc). Since ah-tqc, an error outcome can carry exit code 0. failureComment (internal/reconcile/comments.go:209-214) prints the exit code whenever outcome == domain.OutcomeError, and its own doc comment says 'exit code (only for an error outcome, when the process actually produced one)' โ an invariant ah-tqc silently broke. For the exact live incident ah-tqc exists to fix, the operator now sees on the card: ๐ค attempt 1 failed ยท outcome `error` ยท exit code 0 with no diff stat (there is none) and nothing else. Meanwhile info.FinalErrorMessage โ '401 {"type":"error","error":{"type":"ModelError","message":"Model claude-3-5-haiku is not supported"}}', the one string that makes this diagnosable in five seconds instead of an hour โ goes only to the daemon's log.Warn in runner.go finalizeFromExit. The card is the operator-facing surface and the whole reason the bug mattered; 'error, exit code 0' reads like a daemon bug rather than a broken model config. |
| design | Two changes, one of them cross-package. 1. Gate the exit-code clause in failureComment on exitCode != 0, restoring the invariant its doc comment states. Cheap, local to internal/reconcile/comments.go. 2. Thread FinalErrorMessage to the card. It is parsed in internal/runner (EventStreamInfo.FinalErrorMessage) and needs to reach failureComment, so it crosses ports.RunStatus โ domain.Run โ the comment. Decide deliberately whether it should also be PERSISTED on the run row (useful for /api/v1/status and for a post-hoc audit) or merely passed through to the comment. Persisting means a store migration; passing through means the information is lost on a refinalize, which reads the stored row rather than st. Truncate it โ a provider can return a large body, and SPEC's comment-size discipline applies. Do this AFTER the ah-tqc follow-ups land in internal/runner, since they touch the same struct. |
| acceptance_criteria | A run finalized as error with exit code 0 produces a card comment that names the provider's rejection and does NOT claim an exit code. A run with a genuine non-zero exit still shows it. Covered by a reconcile test asserting both shapes. |
| notes | |
| status | closed |
| priority | 2 |
| issue_type | bug |
| assignee | NULL |
| estimated_minutes | NULL |
| created_at | 2026-08-04T23:42:47Z |
| created_by | Eugene Blikh |
| owner | bigbes@gmail.com |
| updated_at | 2026-08-05T00:22:44Z |
| closed_at | 2026-08-05T00:22:44Z |
| closed_by_session | |
| external_ref | NULL |
| spec_id | |
| compaction_level | 0 |
| compacted_at | NULL |
| compacted_at_commit | NULL |
| original_size | NULL |
| sender | |
| ephemeral | 0 |
| wisp_type | |
| pinned | 0 |
| is_template | 0 |
| mol_type | |
| work_type | |
| source_system | |
| metadata | ๏ฟฝ{} |
| source_repo | |
| close_reason | Done in 8716a2a. Confirmed as filed: failureComment gated the exit-code clause on 'outcome == OutcomeError' alone โ an invariant its own doc comment stated and ah-tqc broke when an error outcome became reachable with exit code 0 โ and the rejection text existed only as EventStreamInfo.FinalErrorMessage inside finalizeFromExit's log.Warn, never crossing the port. Fixed in two places: the clause is now gated on exitCode != 0, and ports.RunStatus carries a new ErrorMessage that applyExit fills for error outcomes only. DESIGN CALL: the text is PERSISTED (domain.Run.ErrorMessage, schema migration adding runs.error_message), not merely passed through, for a reason stronger than the bead's โ finalize derives outcome and exit code from the STORED row on a refinalize, so a pass-through reason would explain a different observation than the outcome printed beside it; and ArchiveWorktree (ah-07g) deletes the worktree the text was parsed from, so after archiving the pass-through source is gone for good. runErrorMessage answers for BOTH Errored() arms: the provider-rejection arm quotes pi verbatim, the no-assistant-turn arm โ which has no provider text โ says the run produced nothing and names the blank-prompt cause. Truncated at 480 RUNES (rune-wise, so multi-byte bodies are never split mid-character) before it leaves the runner, so neither the row nor the card can take a bad gateway's HTML page. /api/v1/status exposes it as error_message,omitempty. Cover: runner_test.go assertions in TestStatusPrecedence and TestStatusReStatFinalizesErroredStream plus TestRunErrorMessage; store_test.go TestRunErrorMessageRoundTrip; three comments_test.go subtests; reconcile_test.go TestFinalizeErrorZeroExitNamesTheReason. FOLLOW-UP: ahub status does not render the new field yet. |
| event_kind | |
| actor | |
| target | |
| payload | |
| await_type | |
| await_id | |
| timeout_ns | 0 |
| waiters | |
| hook_bead | |
| role_bead | |
| agent_state | |
| last_activity | NULL |
| role_type | |
| rig | |
| due_at | NULL |
| defer_until | NULL |
| no_history | 0 |
| started_at | 2026-08-04T23:54:11Z |
| is_blocked | 0 |
| issue_id | ah-1cx.9 |
| label | milestone:v0.2.0 |
| id | ad023105-d4a0-5012-8905-0682ad8ea9d7 |
| issue_id | ah-1cx.9 |
| type | parent-child |
| created_at | 2026-08-05T02:42:46Z |
| created_by | Eugene Blikh |
| metadata | ๏ฟฝ{} |
| thread_id | |
| depends_on_issue_id | ah-1cx |
| depends_on_wisp_id | NULL |
| depends_on_external | NULL |
| id | 019fcf28-8304-7d69-bfa8-d5fdc59279b5 |
| issue_id | ah-1cx.9 |
| event_type | created |
| actor | Eugene Blikh |
| old_value | |
| new_value | |
| comment | NULL |
| created_at | 2026-08-05T02:42:46Z |
| id | 019fcf32-f4d1-7c11-a5ae-97fd94625150 |
| issue_id | ah-1cx.9 |
| event_type | status_changed |
| actor | Eugene Blikh |
| old_value | {"id":"ah-1cx.9","title":"[bug] failure comment renders 'exit code 0' and drops the provider's rejection text","description":"Found by the independent review of 665e805 (ah-tqc).\n\nSince ah-tqc, an error outcome can carry exit code 0. failureComment (internal/reconcile/comments.go:209-214) prints the exit code whenever outcome == domain.OutcomeError, and its own doc comment says 'exit code (only for an error outcome, when the process actually produced one)' โ an invariant ah-tqc silently broke.\n\nFor the exact live incident ah-tqc exists to fix, the operator now sees on the card:\n\n ๐ค attempt 1 failed ยท outcome `error` ยท exit code 0\n\nwith no diff stat (there is none) and nothing else. Meanwhile info.FinalErrorMessage โ '401 {\"type\":\"error\",\"error\":{\"type\":\"ModelError\",\"message\":\"Model claude-3-5-haiku is not supported\"}}', the one string that makes this diagnosable in five seconds instead of an hour โ goes only to the daemon's log.Warn in runner.go finalizeFromExit. The card is the operator-facing surface and the whole reason the bug mattered; 'error, exit code 0' reads like a daemon bug rather than a broken model config.","design":"Two changes, one of them cross-package.\n\n1. Gate the exit-code clause in failureComment on exitCode != 0, restoring the invariant its doc comment states. Cheap, local to internal/reconcile/comments.go.\n\n2. Thread FinalErrorMessage to the card. It is parsed in internal/runner (EventStreamInfo.FinalErrorMessage) and needs to reach failureComment, so it crosses ports.RunStatus โ domain.Run โ the comment. Decide deliberately whether it should also be PERSISTED on the run row (useful for /api/v1/status and for a post-hoc audit) or merely passed through to the comment. Persisting means a store migration; passing through means the information is lost on a refinalize, which reads the stored row rather than st. Truncate it โ a provider can return a large body, and SPEC's comment-size discipline applies.\n\nDo this AFTER the ah-tqc follow-ups land in internal/runner, since they touch the same struct.","acceptance_criteria":"A run finalized as error with exit code 0 produces a card comment that names the provider's rejection and does NOT claim an exit code. A run with a genuine non-zero exit still shows it. Covered by a reconcile test asserting both shapes.","status":"open","priority":2,"issue_type":"bug","owner":"bigbes@gmail.com","created_at":"2026-08-04T23:42:47Z","created_by":"Eugene Blikh","updated_at":"2026-08-04T23:42:47Z"} |
| new_value | {"status":"in_progress"} |
| comment | NULL |
| created_at | 2026-08-05T02:54:11Z |
| id | 019fcf42-165a-7501-b23c-266ecb9bc6f0 |
| issue_id | ah-1cx.9 |
| event_type | label_added |
| actor | Eugene Blikh |
| old_value | NULL |
| new_value | NULL |
| comment | Added label: milestone:v0.2.0 |
| created_at | 2026-08-05T03:10:42Z |
| id | 019fcf4d-1683-7b81-8756-4bb6c3ced5cb |
| issue_id | ah-1cx.9 |
| event_type | closed |
| actor | Eugene Blikh |
| old_value | |
| new_value | Done in 8716a2a. Confirmed as filed: failureComment gated the exit-code clause on 'outcome == OutcomeError' alone โ an invariant its own doc comment stated and ah-tqc broke when an error outcome became reachable with exit code 0 โ and the rejection text existed only as EventStreamInfo.FinalErrorMessage inside finalizeFromExit's log.Warn, never crossing the port. Fixed in two places: the clause is now gated on exitCode != 0, and ports.RunStatus carries a new ErrorMessage that applyExit fills for error outcomes only. DESIGN CALL: the text is PERSISTED (domain.Run.ErrorMessage, schema migration adding runs.error_message), not merely passed through, for a reason stronger than the bead's โ finalize derives outcome and exit code from the STORED row on a refinalize, so a pass-through reason would explain a different observation than the outcome printed beside it; and ArchiveWorktree (ah-07g) deletes the worktree the text was parsed from, so after archiving the pass-through source is gone for good. runErrorMessage answers for BOTH Errored() arms: the provider-rejection arm quotes pi verbatim, the no-assistant-turn arm โ which has no provider text โ says the run produced nothing and names the blank-prompt cause. Truncated at 480 RUNES (rune-wise, so multi-byte bodies are never split mid-character) before it leaves the runner, so neither the row nor the card can take a bad gateway's HTML page. /api/v1/status exposes it as error_message,omitempty. Cover: runner_test.go assertions in TestStatusPrecedence and TestStatusReStatFinalizesErroredStream plus TestRunErrorMessage; store_test.go TestRunErrorMessageRoundTrip; three comments_test.go subtests; reconcile_test.go TestFinalizeErrorZeroExitNamesTheReason. FOLLOW-UP: ahub status does not render the new field yet. |
| comment | NULL |
| created_at | 2026-08-05T03:22:43Z |
No comments.
Close reason