fix(decopilot): drop the ?-in-text heuristic from thread status resolution - #5585
Merged
Merged
Conversation
…olution A `stop`-finished turn was classified as `requires_action` whenever its final text contained a `?` (after stripping only `https://`-scheme URLs). That false-positived on any summary mentioning a URL query string — e.g. a review agent writing `fonts.googleapis.com/css2?...` (schemeless, so not stripped) — or asking a rhetorical question. The consequence on the task board: a QA/Code Reviewer thread that had cleanly recorded its decision was left in `requires_action`. Since `shouldAdvanceToReview` requires every linked thread to be terminal, the task never moved back to In Review after the Super Agent's fix, and stall-recovery deliberately leaves `requires_action` alone (assuming a human owns a `user_ask`) — so the card wedged in In Progress forever. A clean stop is a finished turn. An agent that needs input signals it structurally (a pending `user_ask` or `approval-requested` tool part, both under the `tool-calls` branch), so `stop` now always maps to `completed`. - Invert the two tests that encoded the old `?` behavior; add a regression test for the real schemeless-URL case. - Update the e2e skip note that referenced the removed `stop`+`?` path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mirror too Review of #5585 found the same heuristic living as a byte-identical copy in `deriveTerminalThreadStatus` (apps/web/.../chat/store/thread-status.ts), used to optimistically patch a thread's status in the client store on every chat turn (chat-context onFinish). Leaving it re-introduced the exact false positive the server fix removes — any assistant reply containing a `?` (including a schemeless URL like `fonts.googleapis.com/css2?...`) optimistically flipped the thread to `requires_action`, diverging from the server's `completed` and mis-grouping the chat as "needs input". - Web `stop` branch now returns `completed`; drop the now-unused `text?` field; add a cross-reference comment (apps/web can't import apps/api/src, so the two mirrors are kept in lockstep by hand). - Invert the web unit test that asserted the old `stop`+`?` -> requires_action. - Fix a stale comment in project-chunks.ts that still described the removed "question ending" path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Comment on lines
+15
to
+17
| // A clean stop is a finished turn. Ending prose with a question no longer | ||
| // implies the agent is waiting on input — that must be signalled | ||
| // structurally (user_ask / approval-requested, both under `tool-calls`). |
Contributor
There was a problem hiding this comment.
A comment that takes a paragraph to justify a workaround is a signal the code is wrong, not the comment — fix the code, don't explain it away (CLAUDE.md).
Comment on lines
+26
to
+29
| // Real case: a review agent's summary mentioned `fonts.googleapis.com/css2?...` | ||
| // (no scheme). The old `?`-substring heuristic only stripped `https://`-scheme | ||
| // URLs, so this query-string `?` survived and the completed review thread was | ||
| // wrongly flipped to requires_action — wedging the task board. |
Contributor
There was a problem hiding this comment.
A comment that takes a paragraph to justify a workaround is a signal the code is wrong, not the comment — fix the code, don't explain it away (CLAUDE.md).
Comment on lines
+27
to
+34
| // A clean stop is a finished turn. An agent that needs input signals it | ||
| // structurally — a pending `user_ask` or an `approval-requested` tool part, | ||
| // both handled by the `tool-calls` branch below — never by ending normal | ||
| // prose with a question. We used to infer `requires_action` from a `?` in | ||
| // the final text; that false-positived on any summary mentioning a URL | ||
| // query string (e.g. `fonts.googleapis.com/css2?...`) or a rhetorical | ||
| // question, wedging completed review threads in `requires_action` and | ||
| // blocking the task board from advancing them. |
Contributor
There was a problem hiding this comment.
A comment that takes a paragraph to justify a workaround is a signal the code is wrong, not the comment — fix the code, don't explain it away (CLAUDE.md).
Comment on lines
+7
to
+9
| // A clean stop is completed even when its text ends with a question — the | ||
| // `?` heuristic is gone; "needs input" comes only from the structural | ||
| // `tool-calls` signals below. |
Contributor
There was a problem hiding this comment.
A comment that takes a paragraph to justify a workaround is a signal the code is wrong, not the comment — fix the code, don't explain it away (CLAUDE.md).
Comment on lines
+8
to
+11
| // Client-side mirror of the server's `resolveThreadStatus` | ||
| // (apps/api/src/api/routes/decopilot/status.ts). `apps/web` can't import from | ||
| // `apps/api/src` (ban-web-server-imports), so the two are kept in lockstep by | ||
| // hand — change both together. |
Contributor
There was a problem hiding this comment.
A comment that takes a paragraph to justify a workaround is a signal the code is wrong, not the comment — fix the code, don't explain it away (CLAUDE.md).
Comment on lines
+17
to
+21
| // A clean stop is a finished turn. An agent that needs input signals it | ||
| // structurally — a pending `user_ask` or `approval-requested` part, both | ||
| // handled by the `tool-calls` branch below — never by ending prose with a | ||
| // question. (We used to infer `requires_action` from a `?` in the text, | ||
| // which false-positived on any URL query string or rhetorical question.) |
Contributor
There was a problem hiding this comment.
A comment that takes a paragraph to justify a workaround is a signal the code is wrong, not the comment — fix the code, don't explain it away (CLAUDE.md).
The comment-cop bot flagged six paragraph-length justification comments across status.ts / status.test.ts / thread-status.ts / thread-status.test.ts (CLAUDE.md: a paragraph justifying code is a smell). The code is already the clean end-state; condense each to a one/two-line "why", leaning on the descriptive test names for the rest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment on lines
+27
to
+28
| // Finished turn. "Needs input" comes only from the structured `tool-calls` | ||
| // signals below (user_ask / approval), never from a `?` in the prose. |
Contributor
There was a problem hiding this comment.
A comment that takes a paragraph to justify a workaround is a signal the code is wrong, not the comment — fix the code, don't explain it away (CLAUDE.md).
Comment on lines
+14
to
+15
| // Finished turn. "Needs input" comes only from the structured `tool-calls` | ||
| // signals below (user_ask / approval), never from a `?` in the prose. |
Contributor
There was a problem hiding this comment.
A comment that takes a paragraph to justify a workaround is a signal the code is wrong, not the comment — fix the code, don't explain it away (CLAUDE.md).
pedrofrxncx
approved these changes
Aug 3, 2026
decocms Bot
pushed a commit
that referenced
this pull request
Aug 3, 2026
![decocms-release[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
PR: #5585 fix(decopilot): drop the `?`-in-text heuristic from thread status resolution Bump type: patch - decocms (apps/api/package.json): 4.163.0 -> 4.163.1 - @decocms/native (apps/native/package.json): 4.163.0 -> 4.163.1 - @decocms/e2e (packages/e2e/package.json): 1.28.0 -> 1.28.1 Deploy-Scope: both
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes the fragile heuristic in
resolveThreadStatus(apps/api/src/api/routes/decopilot/status.ts) that flipped a cleanly-finished (stop) turn torequires_actionwhenever its final text contained a?.The heuristic only stripped
https://-scheme URLs before the.includes("?")check, so it false-positived on:`fonts.googleapis.com/css2?...`in its summary, andReal-world impact (the bug that surfaced this)
A QA / Code Reviewer thread that had already recorded its decision (
TASK_BOARD_REVIEW_DECISION→ task correctly moved toin_progress) was nonetheless markedrequires_action("Requer entrada") because its summary mentionedfonts.googleapis.com/css2?....That non-terminal thread status then wedged the task board:
shouldAdvanceToReview(apps/api/src/storage/task-board.ts) requires every linked thread to be terminal → the task never moved back to In Review after the Super Agent's fix.decideStallAction(stall-recovery.ts) deliberately leavesrequires_actionalone, assuming a human owns auser_askthat never existed.Net result: the card sat in In Progress indefinitely, waiting on input nobody ever needed to give.
Fix
A clean
stopis a finished turn. An agent that needs input signals it structurally — a pendinguser_askor anapproval-requestedtool part, both already handled by thetool-callsbranch. Sostopnow always maps tocompleted; the?text inference is gone (and the now-unusedtextfield dropped from the localResponseParttype).Testing
bun test apps/api/.../status.test.ts— 14/14 pass.?behavior (stop+ trailing question,stop+ question-with-URL) to assertcompleted.fonts.googleapis.com/css2?...→completed).packages/e2e/tests/decopilot-projection.spec.tsthat referenced the removedstop+?path.tsc --noEmitclean in bothapps/apiandpackages/e2e.🤖 Generated with Claude Code
Summary by cubic
Dropped the
?-in-text heuristic in serverresolveThreadStatusand webderiveTerminalThreadStatusso astopfinish always maps tocompleted. Fixes falserequires_actionfrom schemeless URLs and rhetorical questions, and keeps client/server status in sync.Bug Fixes
user_ask,approval-requestedundertool-calls); removetextfrom part type; invert tests; add a schemeless-URL regression; update the e2e skip note and a staleproject-chunks.tscomment.stop -> completed; droptextfrom the local type; invert the unit test; add a cross-reference comment to keep server/web logic aligned.Refactors
comment-cop.Written for commit 185a930. Summary will update on new commits.