Skip to content

Commit ec8f74a

Browse files
sahrizviclaude
andcommitted
fix(session): report thinking-only title answers, log the failure without the raw error, and keep error_type identifier-shaped
Bot review findings on the title retry: - An answer with no usable line (empty, or thinking only) is reported as a failed attempt instead of silently using one of the five. - The `failed to generate title` log line carries the error's name, message (capped) and status, not the raw error object, which for a provider error can carry the request body. - `error_type` is accepted only when it is an identifier-shaped token of at most 64 characters; anything else is reported as `Error` or `object`. - When the session cannot be re-read before setting the title, the title is left for a later retry instead of being written blind. - Tests: the in-flight test waits for the held title request to reach the server before the second turn, and the five-attempt test waits for all five failure events before asserting. Co-Authored-By: Claude Fable 5.1 <[email protected]>
1 parent 966518b commit ec8f74a

2 files changed

Lines changed: 30 additions & 16 deletions

File tree

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

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4878,10 +4878,13 @@ NOTE: At any point in time through this workflow you should feel free to ask the
48784878
const code = o?.statusCode ?? o?.lastError?.statusCode
48794879
return typeof code === "number" ? code : undefined
48804880
}
4881-
if (err instanceof Error) return { name: err.name, message: err.message, status: statusOf(err) }
4881+
// The name becomes a telemetry dimension: an identifier-shaped token or nothing.
4882+
const safeName = (value: unknown, fallback: string) =>
4883+
typeof value === "string" && /^[A-Za-z0-9_.:-]{1,64}$/.test(value) ? value : fallback
4884+
if (err instanceof Error) return { name: safeName(err.name, "Error"), message: err.message, status: statusOf(err) }
48824885
if (err && typeof err === "object") {
48834886
const o = err as Record<string, unknown>
4884-
const name = typeof o.name === "string" ? o.name : typeof o.type === "string" ? o.type : "object"
4887+
const name = safeName(o.name, safeName(o.type, "object"))
48854888
let message = typeof o.message === "string" ? o.message : ""
48864889
if (!message) {
48874890
try {
@@ -4989,12 +4992,27 @@ NOTE: At any point in time through this workflow you should feel free to ask the
49894992
} finally {
49904993
titleInFlight.delete(input.session.id)
49914994
}
4992-
if (failure !== undefined || text.trim().length === 0) {
4993-
const err = failure ?? new Error("the model returned no text")
4995+
const cleaned = text
4996+
.replace(/<think>[\s\S]*?<\/think>\s*/g, "")
4997+
.split("\n")
4998+
.map((line: string) => line.trim())
4999+
.find((line: string) => line.length > 0)
5000+
// An answer with no usable line (empty, or thinking only) is a failed attempt too, so it is
5001+
// reported rather than silently using up one of the five.
5002+
if (failure !== undefined || !cleaned) {
5003+
const err = failure ?? new Error(text.trim() ? "the model returned no title line" : "the model returned no text")
49945004
const described = describeTitleError(err)
49955005
const providerID = model?.providerID ?? input.providerID
49965006
const modelID = model?.id ?? input.modelID
4997-
log.error("failed to generate title", { error: err, attempt, providerID, modelID })
5007+
// Name, message and status only: a provider error object can carry the request body.
5008+
log.error("failed to generate title", {
5009+
errorType: described.name,
5010+
message: described.message.slice(0, 500),
5011+
status: described.status,
5012+
attempt,
5013+
providerID,
5014+
modelID,
5015+
})
49985016
Telemetry.track({
49995017
type: "title_generation_failed",
50005018
timestamp: Date.now(),
@@ -5008,19 +5026,12 @@ NOTE: At any point in time through this workflow you should feel free to ask the
50085026
})
50095027
return
50105028
}
5011-
// altimate_change end
5012-
const cleaned = text
5013-
.replace(/<think>[\s\S]*?<\/think>\s*/g, "")
5014-
.split("\n")
5015-
.map((line: string) => line.trim())
5016-
.find((line: string) => line.length > 0)
5017-
if (!cleaned) return
50185029

50195030
const title = cleaned.length > 100 ? cleaned.substring(0, 97) + "..." : cleaned
5020-
// altimate_change start — the user may have named the session while the request was in flight;
5021-
// a generated title only ever replaces the placeholder.
5031+
// The user may have named the session while the request was in flight: a generated title only
5032+
// ever replaces the placeholder, and when that cannot be checked the title is left for a retry.
50225033
const current = await Session.get(input.session.id).catch(() => undefined)
5023-
if (current && !Session.isDefaultTitle(current.title)) return
5034+
if (!current || !Session.isDefaultTitle(current.title)) return
50245035
// altimate_change end
50255036
return Session.setTitle({ sessionID: input.session.id, title })
50265037
}

‎packages/opencode/test/session/title-retry.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,9 @@ it.live(
232232
parts: [{ type: "text", text: `message ${i}` }],
233233
})
234234
}
235-
// Two requests per turn for the first five turns, one for the sixth.
235+
// Two requests per turn for the first five turns, one for the sixth, and all five failures recorded.
236236
yield* llm.wait(11)
237+
for (let i = 0; i < 150 && events.length < 5; i++) yield* Effect.sleep("20 millis")
237238

238239
const titleHits = (yield* llm.hits).filter((hit) => JSON.stringify(hit.body).includes(TITLE_MARKER))
239240
expect(titleHits).toHaveLength(5)
@@ -308,6 +309,8 @@ it.live(
308309
agent: "build",
309310
parts: [{ type: "text", text: "first message" }],
310311
})
312+
// The held title request has reached the server before the second turn starts.
313+
yield* llm.wait(2)
311314
yield* prompt.prompt({
312315
sessionID: session.id,
313316
agent: "build",

0 commit comments

Comments
 (0)