Skip to content

Commit e9bde73

Browse files
anandgupta42claude
andcommitted
fix(harness): second-wave review fixes — completion detector and run accounting hardening
- `termination.ts`: a fence-looking line with trailing info-string text (e.g. ```` ```not-a-closer ````) was treated as a valid closer for an already-open code fence; only a run of the same/longer marker followed by nothing but whitespace may close a fence now, matching CommonMark, so a still-open fence's interior `DONE` can no longer terminate a run. - `run-accounting.ts` / `run.ts`: the idle-done confirm-DONE challenge's two abort/finish suppression flags could both still be "fresh" once the challenge reply itself was sent (the interrupted prompt's abort may surface via only one of the two channels), letting a genuine failure of the challenge reply be silently forgiven. `onIdleDoneChallengeReplySent()` now marks the reply as in flight so a later abnormal signal is scored as a real failure, not absorbed by suppression meant for the earlier abort. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_017KXpxBn4zteNfXTwv8cf93
1 parent c49df38 commit e9bde73

5 files changed

Lines changed: 80 additions & 6 deletions

File tree

‎packages/opencode/src/cli/cmd/run-accounting.ts‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,14 @@ export namespace RunAccounting {
7474
// finishes are real failures.
7575
let challengeAbortSuppressed = false
7676
let challengeFinishSuppressed = false
77+
// altimate_change start — upstream_fix: the abort of the interrupted prompt
78+
// can surface as onSessionError(MessageAbortedError), onPromptResult
79+
// (finish="error"/"other"), or both — either channel may fire for that
80+
// SAME abort, so both suppressions above are scoped to it. Once the
81+
// challenge reply itself is sent, a real failure there (e.g. an errorless
82+
// finish="other" on the confirm-DONE reply) must not be silently forgiven
83+
// by whichever suppression the interrupted prompt's abort left unused.
84+
let challengeReplySent = false
7785

7886
function isCompactionStep(messageID: string) {
7987
return agents.get(messageID) === "compaction"
@@ -112,14 +120,25 @@ export namespace RunAccounting {
112120
onIdleDoneChallengeIssued() {
113121
idleDoneChallengeTurn = turnCount
114122
},
123+
// altimate_change start — upstream_fix: see challengeReplySent above.
124+
/** the idle-done confirm-DONE challenge reply has been sent; suppression of the interrupted prompt's own abort no longer applies. */
125+
onIdleDoneChallengeReplySent() {
126+
challengeReplySent = true
127+
},
128+
// altimate_change end
115129
onSessionError(name: unknown, message?: string) {
116130
const errorName = typeof name === "string" && name.length > 0 ? name : "UnknownError"
117131
if (RECOVERABLE_ERROR_NAMES.has(errorName)) return
118132
// the idle-done challenge is delivered by aborting the in-flight
119133
// prompt first; that harness-initiated abort surfaces as a
120134
// MessageAbortedError and must not be scored as a fatal run error.
121135
// Exactly ONE such abort exists per challenge — later aborts are real.
122-
if (idleDoneChallengeTurn !== undefined && !challengeAbortSuppressed && errorName === "MessageAbortedError") {
136+
if (
137+
idleDoneChallengeTurn !== undefined &&
138+
!challengeAbortSuppressed &&
139+
!challengeReplySent &&
140+
errorName === "MessageAbortedError"
141+
) {
123142
challengeAbortSuppressed = true
124143
return
125144
}
@@ -151,7 +170,7 @@ export namespace RunAccounting {
151170
// the terminal message of the ONE prompt the idle-done fallback
152171
// aborted (to deliver its challenge) finishes abnormally by design;
153172
// any further abnormal finish is a real failure.
154-
if (idleDoneChallengeTurn !== undefined && !challengeFinishSuppressed) {
173+
if (idleDoneChallengeTurn !== undefined && !challengeFinishSuppressed && !challengeReplySent) {
155174
challengeFinishSuppressed = true
156175
return
157176
}

‎packages/opencode/src/cli/cmd/run.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,6 +1115,12 @@ You are speaking to a non-technical business executive. Follow these rules stric
11151115
// delivered via the nudge arbiter so this injected turn carries exactly
11161116
// ONE system-authored directive.
11171117
if (idleDone.challengeIssued && !accounting.fatal) {
1118+
// altimate_change start — upstream_fix: mark the challenge reply as
1119+
// sent BEFORE anything in this phase can raise a session-error/prompt-
1120+
// result event, so a genuine failure of the reply itself is never
1121+
// absorbed by the interrupted prompt's own abort suppression.
1122+
accounting.onIdleDoneChallengeReplySent()
1123+
// altimate_change end
11181124
// Dedicated abort for the challenge subscription so a failed challenge
11191125
// send can cancel the event-stream loop deterministically (the SSE
11201126
// generator exits cleanly on abort; the loop's for-await then drains).

‎packages/opencode/src/session/termination.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,24 @@ export namespace SessionTermination {
4444
if (last.replace(/^ {0,3}/, "") !== DONE_TOKEN) return false
4545
// Reject a final line inside an unclosed code fence — the block's content is
4646
// quoted material, not an assertion. Fence state follows CommonMark: a fence
47-
// opens with a run of >= 3 backticks or tildes; only a run of the SAME
48-
// character with at least the SAME length closes it. Any other fence-looking
49-
// line inside an open fence (other marker, or a shorter run) is content.
47+
// opens with a run of >= 3 backticks or tildes (an info string, e.g. an
48+
// opening ```lang, is permitted); only a run of the SAME character with at
49+
// least the SAME length, followed by nothing but optional whitespace, closes
50+
// it. A fence-looking line with a trailing info string is opener/content,
51+
// never a valid closer — treating it as one would let a still-open fence's
52+
// interior DONE terminate the run.
5053
let open: { char: string; length: number } | undefined
5154
for (let i = 0; i < lines.length - 1; i++) {
5255
const match = CODE_FENCE_PATTERN.exec(lines[i]!)
5356
if (!match) continue
5457
const marker = match[1]!
5558
if (!open) {
5659
open = { char: marker[0]!, length: marker.length }
57-
} else if (marker[0] === open.char && marker.length >= open.length) {
60+
} else if (
61+
marker[0] === open.char &&
62+
marker.length >= open.length &&
63+
/^[ \t]*$/.test(lines[i]!.slice(match[0]!.length))
64+
) {
5865
open = undefined
5966
}
6067
}

‎packages/opencode/test/cli/run-accounting.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,4 +335,36 @@ describe("RunAccounting done_reason + idle-done bookkeeping", () => {
335335
expect(acc.termination().why_harness_stopped).toBe("error")
336336
expect(acc.fatal).toBe(true)
337337
})
338+
339+
// altimate_change start — upstream_fix regression: the interrupted prompt's
340+
// abort can surface as EITHER onSessionError(MessageAbortedError) or an
341+
// abnormal onPromptResult (or both) — both suppressions above are scoped to
342+
// that one abort. Once the challenge reply itself has been sent, a genuine
343+
// failure of the reply must not be absorbed by whichever suppression the
344+
// interrupted prompt's abort left unused.
345+
test("a genuine challenge-reply failure is fatal even if the interrupted prompt's abort only used one suppression channel", () => {
346+
const acc = RunAccounting.create()
347+
acc.onIdleDoneChallengeIssued()
348+
// The interrupted prompt's abort surfaces ONLY via onSessionError — its
349+
// own onPromptResult never reports an abnormal finish (e.g. it resolved
350+
// "stop" before the abort landed), so challengeFinishSuppressed is never
351+
// consumed here.
352+
acc.onSessionError("MessageAbortedError", "aborted")
353+
expect(acc.fatal).toBe(false)
354+
// The challenge reply is now sent; its own errorless abnormal finish is a
355+
// real failure of the confirmation, not a leftover of the abort above.
356+
acc.onIdleDoneChallengeReplySent()
357+
acc.onPromptResult({ finish: "other" })
358+
expect(acc.fatal).toBe(true)
359+
expect(acc.termination().why_harness_stopped).toBe("error")
360+
})
361+
362+
test("both suppression channels still forgive the same interrupted-prompt abort before the reply is sent", () => {
363+
const acc = RunAccounting.create()
364+
acc.onIdleDoneChallengeIssued()
365+
acc.onSessionError("MessageAbortedError", "aborted")
366+
acc.onPromptResult({ finish: "error" })
367+
expect(acc.fatal).toBe(false)
368+
})
369+
// altimate_change end
338370
})

‎packages/opencode/test/session/termination.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,16 @@ describe("SessionTermination.isExplicitDone", () => {
5757
expect(SessionTermination.isExplicitDone("~~~\n```\n~~~\nDONE")).toBe(true)
5858
})
5959

60+
test("a fence-looking line with trailing info-string text never closes the fence", () => {
61+
// Per CommonMark, only whitespace may follow a closing fence's marker —
62+
// a line like "```not-a-closer" is content (or a nested opener), not a
63+
// closer. The DONE that follows is still inside the open fence.
64+
expect(SessionTermination.isExplicitDone("```\ncode\n```not-a-closer\nDONE")).toBe(false)
65+
expect(SessionTermination.isExplicitDone("~~~\ncode\n~~~lang\nDONE")).toBe(false)
66+
// A real closer with trailing whitespace only still closes normally.
67+
expect(SessionTermination.isExplicitDone("```\ncode\n``` \nDONE")).toBe(true)
68+
})
69+
6070
test("quoted and indented-code DONE never terminates", () => {
6171
expect(SessionTermination.isExplicitDone("The instructions said:\n> DONE")).toBe(false)
6272
expect(SessionTermination.isExplicitDone("Example:\n DONE")).toBe(false)

0 commit comments

Comments
 (0)