fix(daemon): answer client-fixable queue park/close refusals with 400 instead of 500

[rudycelekli]

What a user gets

Before: POST /api/queue/:id/update refusals that the caller can fix themselves were answered with HTTP 500, because the route's errorResponse map did not know their codes. A wrong --wake-after, a wake option on a transition that is not a park, blocked_on or closure fields on a transition that cannot store them: the body already named the right code and message, but the status said "daemon fault" and the CLI exited 2 (5xx) instead of 1 (4xx). After: these answer 400 with the same body.

Mapped to 400: closure_fields_not_admitted, blocked_on_not_admitted, wake_not_admitted, wake_ambiguous, wake_after_invalid, wake_max_invalid, wake_message_not_admitted, wake_message_invalid, wake_watchdog_not_live, wake_watchdog_target_mismatch, wake_self_blocker.

Left at 500 on purpose:

• wake_intent_store_unavailable, outbox_db_mismatch, terminal_close_without_wake_intent: they describe the daemon's own state.
• blocker_not_found, blocker_not_live, blocker_malformed: they are client-side too, but packages/cli/test/queue-update-metadata-reject-cli.test.ts pins their CLI exit code 2 (real CLI against the real route). I did not want to change that contract in a status-mapping PR; say if you want them moved too and I will update that test with them.

How you verified it

Tested at upstream main (fetched and rebased) plus this change, Node 22, npm ci --ignore-scripts -w packages/daemon -w packages/cli, daemon built with npm run build -w packages/daemon for the CLI test.

New packages/daemon/test/queue-client-error-status.test.ts drives the real queueRoutes + QueueRepository over app.request (5 refusals, each asserts the named code and status 400):

before the fix (src reverted): Tests 5 failed
   {"error":"wake_not_admitted","message":"a park wake persists only with state=blocked; ..."}: expected 500 to be 400
   (same shape for wake_ambiguous, wake_after_invalid, blocked_on_not_admitted, closure_fields_not_admitted)
after the fix:                 Tests 5 passed

Regression runs after the change: all 29 daemon queue-* / sweep-a-closure-coherence / gateway-human-queue-journey test files, 676 passed; the 8 CLI test files that mention wake/blocked-on/closure flags (incl. queue-update-metadata-reject-cli, queue.test, wake-after-invalid-duration), 317 passed; tsc --noEmit -p packages/daemon/tsconfig.json clean. (While drafting I had also mapped the three blocker_* codes and the CLI test went red, 6 failures with exit 1 instead of 2; that is why they are excluded.)

Not run: npm run lint for the other packages, the full npm test, a live daemon.

Anything you were unsure about

• Whether blocker_* should join the 400 group (see above); and whether the wake_watchdog_* pair should be 409 rather than 400. I used 400, matching the existing reply_to_not_found and invalid_state entries.

• wake_max_invalid, wake_message_* are not reachable through the route's current body fields, so they are mapped for completeness but not covered by the HTTP test.

• Open PR feat(cli): store an outcome with rig queue update --state <closing> --resolution <text> #1292 adds three codes to the same chain in a different hunk; I put mine after workflow_frontier_packet to avoid a textual conflict.

• One concern per PR; no version bump; no CHANGELOG.md edit

• Tests added or updated where the change is testable

• I listed the checks I ran, their results, and any checks I could not run

(Checkboxes left for the author to tick.)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Queue update requests with invalid or unsupported closure, blocker, wake, or watchdog settings now return HTTP 400 instead of an internal server error.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9a03f432-e2dc-446b-891f-c31386e16bfd

📥 Commits

Reviewing files that changed from the base of the PR and between 63f7b4c and 2f3d3d6.

📒 Files selected for processing: 2

• packages/daemon/src/routes/queue.ts
• packages/daemon/test/queue-client-error-status.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 2 remain after this review.

📝 Walkthrough

The queue update route now returns HTTP 400 for additional inadmissible field and validation errors. Integration tests verify HTTP 400 responses and error codes for five refusal cases.

Changes: Queue error status mapping

Layer / File(s)	Summary
Map queue validation errors and test responses packages/daemon/src/routes/queue.ts, packages/daemon/test/queue-client-error-status.test.ts	errorResponse maps additional closure, blocker, wake, and watchdog validation errors to HTTP 400. Integration tests check the response status and error code for five refusal cases.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: mvschwarz

Merge Risk: ⚪ Minimal · up to 2f3d3

No merge-blocking issue is identified; the queue refusals can return the intended client-error status.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main change: client-fixable queue park and close refusals now return HTTP 400 instead of HTTP 500.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mvschwarz]

Thanks, @rudycelekli. This makes eleven caller-fixable queue update refusals answer 400 instead of 500, with the same body. We'll look once it's marked ready for review.

[mvschwarz]

Thanks again, @rudycelekli. In our checks, update refusals that the caller can fix by changing or refreshing the request now answer 400 instead of 500, with the response body unchanged, so rig queue update exits 1 for them instead of 2. Keeping the three blocker_* codes out of this PR is fine, and 400 is fine for the wake_watchdog_* pair, which answers your question about 409.

A few small things, please:

1. Merge current main into the branch. This PR's CI ran against an older main, and two PRs merged since, feat(cli): show a closed queue row's closing note and reason in rig queue show --full #1290 and feat(cli): store an outcome with rig queue update --state <closing> --resolution <text> #1292, changed routes/queue.ts. Their changes are in other parts of the file and merge without conflicts, but CI hasn't run on the combined code.
2. The PR description:
   • Please state the counts as: this PR maps eleven codes to 400; seven of them are refusals the update route can return today; six rig queue update inputs now exit 1 instead of 2.
   • In the description's "Before" paragraph, a wrong --wake-after is refused by the CLI itself before it sends anything, with exit 1, so only a raw HTTP request got the 500.
3. The test comment at the top of queue-client-error-status.test.ts: please drop "a ghost blocker" (blocker_not_found), since that refusal still answers 500 in this PR.

Once CI has run on the combined code, we'll look at that run and follow up here.