Skip to content

fix(cli): record runtime post refusals - #1246

Merged
lilyshen0722 merged 1 commit into
fix/prose-overflow-threads-not-attachmentsfrom
fix/wrapper-refusal-delivery
Aug 26, 2026
Merged

fix(cli): record runtime post refusals#1246
lilyshen0722 merged 1 commit into
fix/prose-overflow-threads-not-attachmentsfrom
fix/wrapper-refusal-delivery

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Summary

  • Treat normal HTTP-200 { refused: true } responses from the runtime message route as a non-delivery in every chat-delivery rung: single, split, thread root, thread continuation, attachment card, and attachment fallback.
  • Stop on the first refusal, preserve the number of chunks that actually landed, surface server guidance locally, and ack the event as no_action instead of posted so it is not replayed unchanged.
  • Bump @commonlyai/cli to 0.1.20.

Stack

Verification

  • cd cli && npm test -- --runInBand — 24 suites passed, 348 tests passed, 10 skipped.
  • cd cli && npm test -- --runInBand __tests__/enforcement.test.mjs __tests__/run-loop.test.mjs — 128 passed.
  • Mutation: replacing the thread-continuation refusal return with a normal thread result made only its paired refusal test fail.
  • Mutation: bypassing the run-loop refusal branch made only its paired ack test fail.
  • node --check cli/src/lib/enforcement.js and node --check cli/src/commands/agent.js; git diff --check.
  • npm run lint is unavailable in this package because its script calls eslint but eslint is not installed.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Gated at e92d6c8b, 22:28:24Z — OPEN/CLEAN, all checks passing. Approve. Three notes, none blocking, and one of them matters because TASK-071 is already marked done.

Verified

Stacked correctly. git merge-base origin/pr1246 origin/pr1217 = 83f8ae38, exactly #1217's head. Diff against that base is 285 insertions / 15 deletions across five files, so I'm reviewing this change and not #1217's.

The mechanism is right, and it is right at the boundary rather than per-branch. Hoisting postMessage and a shared refused() builder means all six delivery branches share one predicate, so the next branch someone adds inherits it. That is the correct altitude for this fix — a per-call-site check would have been the thing the next editor forgets.

Branch coverage is complete. Seven new tests, one per delivery branch — single, split, thread root, thread continuation, attach card, attach fallback — plus the run-loop ack. I checked each branch in the source has a matching refusal return; none is missed.

The partial-count arithmetic is correct on every branch I traced. split and split-fallback now increment a local messages and return it rather than chunks.length; thread-fallback gained the posted += 1 it was missing, so its count is no longer silently short. The non-refused returns still report chunks.length, which is right because posted has reached it by then.

Your lint admission checks out, and is worse than stated — see note 3.

1. The partial post survives, and the row is closed

With the cap at 3 and one prior consecutive message, a 3-chunk split posts chunks 1–2 and is refused on chunk 3. This PR makes the seat learn that — which was the entire defect, so this is the right first step — but the pod is still left showing a reply truncated mid-thought, and the server guidance expressly says not to retry unchanged. So the visible symptom becomes reported rather than prevented.

That is a defensible place to stop. My concern is only that TASK-071 is marked done, so the remaining half now has no owner. Worth making it an explicit decision — either "reporting is sufficient, the seat can recover" or a follow-up row — rather than something that quietly falls off.

2. A refused attach can orphan an upload

In the attach branch the file is uploaded via client.upload before the card is posted. if (response?.refused === true) return refused(response, 0, 1) now returns with the upload already stored and no message referencing it. Previously the post always appeared to succeed, so this is new. Probably acceptable — an orphaned blob is cheaper than a lost reply — but it is a real consequence and I did not see it named anywhere.

3. cli/src is entirely unlinted, by both paths

Your note says cli/package.json invokes eslint but does not install it. Confirmed: "lint": "eslint src/", and eslint appears in neither dependencies nor devDependencies. The part not stated is that the other path does not cover it either — root npm run lint is npm run lint:backend && npm run lint:frontend, and there is no eslint config file at the root or anywhere under cli/.

So this is not "lint is temporarily unavailable in this workspace", it is "the CLI has never been linted." That is a sibling of TASK-024 (backend eslint reaching 0 of 286 .ts files) and deserves its own row rather than a line in a PR body. Not this PR's job to fix.

Scope note on the claim

The task notes say "every wrapper post path". The code covers every chat delivery path. agent.ask responds through /asks/:id/respond (agent.js:1080), a different route — I did not check whether that route is subject to the run cap, so I am not claiming a gap, only that the note's phrasing is broader than what I verified.

Not verified

  • I did not run the suite. Your 348 passed / 10 skipped and the mutation results are unchecked by me; CI is green at this head.
  • Whether consecutiveSpawnFailures = 0 is the right outcome for a refusal. stoodDown covers cascade-cap and claim-held but not a delivery refusal, so the streak resets. I think that is correct — the model turn and the HTTP request both completed, and the refusal is server policy rather than a broken delivery path — but the comment above it sets the bar at "the delivery path recovered", which a refusal does not obviously meet. Flagging it as a judgment call you may have already made rather than as a defect.

@lilyshen0722
lilyshen0722 merged commit fd8c907 into fix/prose-overflow-threads-not-attachments Aug 26, 2026
4 checks passed
@lilyshen0722
lilyshen0722 deleted the fix/wrapper-refusal-delivery branch August 26, 2026 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant