Repository navigation
fix: answer a repeated verbatim done with its stored receipt - #45
Merged
Merged
Conversation
A verbatim done moves its task to done in the same request that posts it, so the retry that reuses the deterministic milestone id must answer the receipt the first post stored instead of the 401 a dead task token earns. A completion that commits and then fails on both passes must keep that posted receipt: the last-resort narrated fallback erases the ts the thread already has and tells a retry the milestone was never posted.
A verbatim done moves its task to done in the same request that posts it, so the task token died before its own milestone could be retried: a runner whose answer was lost re-posted the same deterministic id and got 401, then reported an outcome the delegation route refuses. A done task's token now authorizes only the milestone whose posted receipt it wrote, which is what the receipt's new task_id column records: the retry settles on the stored receipt and makes no Slack call, while a new id, another task's receipt and GET /progress/replies keep 401. The last-resort narrated fallback also ran after store.complete had posted the receipt and moved the task, erasing the ts the thread already has; it now writes only while the receipt is still in flight.
Resolving the done replay needs the milestone id, so the handler started parsing the body before authorizing it: an unauthenticated request with a malformed body answered 400 invalid_request where the base answered 401. The body is still read first to learn the replay id, but authorization now decides the answer before the body's validity does. Closes the A1 review P1 on the progress handler's refusal order.
…done-replay # Conflicts: # apps/slack-agent/test/migrations.test.ts
…taging merge Staging added the status length refusal to the milestone handler while this branch moved the body read ahead of authorization, and together they push handle to a complexity of 21, which fails the lint gate on the merged tree. The pre-claim refusals move into one function with the same order and answers.
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
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.
A verbatim
donemilestone moves its task todonein the same request that posts it, so the task token died before its own milestone could be retried: a runner whose answer was lost re-posted the same deterministic id and met 401unauthorized, then reported an outcome the delegation route refuses. A done task's token now authorizes only the milestone whose posted receipt it wrote, so the retry settles on that receipt (200 {"ok": true, "ts": "172.1"}) without a Slack call; a new id, another task's receipt andGET /progress/replieskeep 401.The last-resort narrated fallback also ran after
store.completehad posted the receipt and moved the task, erasing the ts the thread already has and telling the retry the milestone was never posted; it now writes only while the receipt is still in flight.Verify
200 {"ok": true, "ts": "172.1"}with no new Slack call:a repeated verbatim done answers its stored receipt and no new milestone passes the dead tokenasserts the stored receipt, an unchangedchat.postMessagecount and an unchanged total Slack call count. Red on f28de36 (Expected: 200 / Received: 401at the second post), green after.{"error": "unauthorized", "ok": false}fordone-2, with no receipt row for it and no Slack call. It is a guard, green before and after.another task's posted receipt stays unauthorized once its task is done(two tasks, both done, the second task's token replaying the first's milestone id). Droppingtask_id = delegation_tasks.idfrom the replay lookup turns its answer into 403token_scope, so the binding is what the test pins.a verbatim done whose completion keeps failing after the commit keeps its posted receipt, applied as the first test next toa verbatim done whose first completion pass fails after the commit still finishes the task. Red on f28de36: the receipt came back{"state": "narrated", "ts": null}where the batch had storedposted172.1; green after.complete()twice converges:running the done completion twice converges on one posted receipt and one done taskruns the completion three times (the replay, then a directstore.completewith a different ts) and asserts onepostedreceipt holding the ts the first completion stored, onedonetask, no second Slack call. Red on f28de36 (the second post 401), green after.bun run verify: exit 0, 376 pass / 0 fail across 17 files, format, lint, typecheck, knip,deploy:dryandbun audit(561 packages, 1 ignored) green.Commits
testf244b81: pins the replay, the dead-token refusals and the kept receipt.fixdcb1b2b: thecompletedTaskauthorization, the receipt's task binding and the in-flight guard on the narrated fallback.Migration
0010_progress_receipts_task.sqladdsprogress_receipts.task_idbecause the replay must know which task a posted receipt belongs to, and the receipt stored no such binding: without it a done task's token could replay any posted receipt, since a milestone id alone does not say who wrote it.delegation.store.completewrites it in the same batch that moves the task to done, so a posted receipt carrying a task binding implies the task is done by that token.Not covered
POST /progressnow parses the body before it authorizes, so a malformed body with a dead token answers 400invalid_requestwhere it answered 401: the milestone id decides whether a done token may replay and the body is the only place it appears. No store or Slack call happens before either refusal.Goal:
bloop-delegation-3row A1.