Repository navigation
Allow requesters to cancel unaccepted task results - #203
Conversation
hsliuustc0106
left a comment
There was a problem hiding this comment.
Verifier review — INC
Verdict: INC — no actionable findings and every check I ran is green, but the repo's own merge contract is not yet satisfied: this is a src/host/ change whose smoke test has not run, and no independent (Codex) review exists on this tip. Per the review-pr skill, one-side-only review cannot be PASS.
Tip SHA: a07ed3b2f7ce1d8748093c02e84cc66e4dcc4038
Ran (in a worktree at this tip, Node 22.17.0):
- TZ=UTC npm run verify → pass (core 755, social 160, tree 56, .github scripts 56; oxlint + arch-docs "OK (7 changed file(s))"; build-client --check; ds-bot-patches check)
- Fail-without-change: reverted only core/src/host/tasks.js to merge base a63309f, ran test/tasks.test.js → 2 of the 5 new tests fail with exactly "a task that is result-ready cannot become cancelled" (the transition tests); restored the tip file
- Byte-compared core/src/host/tasks.js and core/test/tasks.test.js against patches/ds-bot/files/ counterparts → identical
Not run:
- Smoke test — this is a src/host/ change, so AGENTS.md asks for it; the author deferred it pending the DSH dsh-v0.2.0-rc.2 build, and this machine has no ../deepseek-harness checkout to run the bench
- Desktop, real-model steward, live OpenCode/Bot Social integration — out of scope for this change
- Codex-side review — none exists on this tip yet
OpenCode↔Codex: only OpenCode
Verdict: INC
What the change is
The entire host change is one transition-table entry (result-ready: ['accepted', 'cancelled']) plus a rewritten comment at core/src/host/tasks.js:27. The task-cancel endpoint is untouched: authority still comes from the server-derived actor via requireRequester (body claims like requesterId/actorId are ignored, and the forged-actor test confirms bot/forbidden/bot/unauthorized), terminal states stay terminal, and transition() only mutates status/actors — so the returned summary and actor trail survive cancel and restart by construction. That matches issue #202's acceptance list point by point, including "task disposition, not runtime-stop confirmation," which the two doc updates state correctly.
Findings
No actionable findings. Notes, none blocking:
- The other three new tests (forged/revoked actors, accept-vs-cancel race) also pass on the pre-change host — they pin pre-existing refusal behavior of the new pathway rather than the fix itself. The PR body's claim ("the cancellation regression failed… then passed") is accurate as written, so this is hardening, not dishonest coverage.
- CI's
tests-okand thepackagejob are green on this exact SHA. - Local-only wrinkle:
plugins/socialfails 3 face-room title tests on a UTC+8 machine; underTZ=UTCthey pass 160/160. Environmental, unrelated to this PR.
Security fence / display names
Not in scope — the diff touches no delivery, fencing, or model-visible remote text. The task.result.summary storage (2,000-char cap) predates this PR; cancel adds no new model-visible surface. No secrets, no DSH patching, no hand-edited client.js, tests use the plugin's temp homes.
This report is a Verifier assessment only — not merge authority. To reach PASS the maintainer needs the smoke test (or a written waiver for it) and an independent review agreeing on this same tip SHA; if the tip moves, this review is void.
Fixes #202. Refs #198.
A returned task currently has only an acceptance path, so the requester cannot cancel an unwanted result. Allow
result-ready → cancelledthrough the existingtask-cancelendpoint. Preserve the returned summary and requester actor history, and keep accepted/cancelled/failed states terminal. This records task disposition, not confirmation that an external runtime stopped.Five new channel regressions cover persistence, late transitions, forged/wrong/revoked actors, the requester of a granted Bot versus its owner, and acceptance racing cancellation. Update subsystem/design docs, the changelog fragment and generated patches. Independent of #201.
Verification at
a07ed3b2f7ce1d8748093c02e84cc66e4dcc4038:DSH_HARNESS=/workspace/FlyBot/core/dev/harness npm run verify: pass, 1,027 tests, 0 failed/skipped.dev/bench.sh task-cancel 3296 8896 resetwithCONTEXT_WINDOW=100000 CHECK_RATIO=0.45, thennode dev/smoke.mjs task-cancel 3296in core/. DSH source pinned to tagdsh-v0.2.0-rc.2(639ed015397290b3745d163aafe02ffee4aa3f84), built without source changes. Both bench processes stopped.result-ready cannot become cancelled.No new endpoint, runtime privilege, social friendship exception or team-mode enablement.
Smoke evidence at
a07ed3b2f7ce1d8748093c02e84cc66e4dcc4038The smoke evidence missing from the INC report is now available below. These are author-run results, not an independent review. The tip is unchanged,
tests-okis green, and independent review remains outstanding.From
core/, with the built DSH checkout at tagdsh-v0.2.0-rc.2(639ed015397290b3745d163aafe02ffee4aa3f84):Captured output: all 40 steps passed (mock model only)
Output SHA-256:
c6f7db76d7d9508e4f34d510353d120a950fe10cb90561624dd19f43cea98ea1. Cleanup confirmed both mock and web processes stopped.