fix(tasks): a conflicting write stands down under another seat's live lease (AX 61)

[lilyshen0722]

The defect (AX entry 61)

POST /:podId/:taskId/complete writes on { status: { $in: ['claimed', 'pending'] } } — there is no lease term anywhere in the write. A seat could therefore complete a row another seat was actively working, and the response reported status: done beside a claimedBy whose lease was still thirty minutes out. Measured: claim at 12:15:08.569Z, completion at 12:15:18.875Z, same row.

PATCH /:podId/:taskId was the wider hole of the two: its filter is { podId, taskId } alone, which is how the board editor and the openclaw commonly_update_task retitle or reassign a row.

The fix

One shared refusal for both conflicting write paths. A write is refused only when a different seat holds an unexpired lease, and the 409 carries the fields claim already returns on a lost race (claimedBy, status, claimExpiresAt) — so a refused writer learns who holds the row and when it frees, the same two things a losing claimant learns.

It is expressed through the existing deriveLeaseState label rather than a fourth copy of the expiry rule. Two consequences, each pinned by a test:

• Not a lapsed lease. Refusing on "a lease exists" would strand work behind a dead seat — the objection that kept this out of the kernel. A lapsed lease is claimable again by the existing CAS, and recovery stays lazy.
• Not claimableConditions. That predicate has no branch for status: 'done', so a claimability-based guard would have frozen every finished row against a retitle or an after-the-fact prUrl.

Deliberately not gated: POST /:podId/:taskId/updates. A note on someone else's row is the coordination a claim exists to enable, and for the holder it is the renewal path (#1080 part 1). That handler already draws the line correctly — the note lands either way, only the lease extension is holder-gated.

Humans are never refused: a lease is seat coordination, not authority. isSeatCaller is the union of the two auth shapes (req.agentUser, req.user.isBot), so a person completing or correcting a row is unaffected.

Proof

16 tests in backend/__tests__/unit/routes/tasksApi.leaseGuard.test.js, against a real in-memory Mongo, because the holder match is a Mongo filter rather than a mocked model. Six mutations, each run alone:

mutation	red
no seat is ever refused	4 (including the composition case below)
keyed on "a lease exists" instead of "unexpired"	3 (the lapsed row, the stale legacy claim, the done-row PATCH)
caller comparison dropped	2 (the holder must stay able to write its own row)
human exemption dropped	2
note-append gated too	1
bot-JWT limb dropped (Boolean(req.agentUser) alone)	1 — found in gate, not by me

The last row is the instructive one. That mutation left the suite green at the head I submitted: x-test-username (the header the auth mock turns into isBot) was written into the harness and never sent by any caller, so req.user.isBot was false in every test and the guard's third caller shape was a vacuous antecedent. The branch was correct; nothing would have noticed if it stopped being. There is now a helper named for the shape (asBotJwt), because the fix is to construct the caller, not to comment the branch.

This is a coordination guard, not a lock. The read and the write that follows it are not atomic — a lease can go live in the window between leaseRefusalForWrite and findOneAndUpdate. That is the deliberate trade: the failure it prevents is a seat writing through a live claim, and the failure it tolerates is a rare narrow race that the next write resolves. Read it as "a conflicting write stands down", never as mutual exclusion.

This branch was rebased onto main after #1767 merged, and the suites re-run there: 113/113 across the 8 tasksApi and guard suites, 30/30 on the gate's own baseline set, plus 155/155 across the wider set that stubs Task (including routeRateLimitGuard, run per the clause in backend/TESTING.md:85, although this touches no route registration and no limiter). lint:ts 0 errors.

Composition with #1767 (now merged)

#1767 added await resolveTaskAttention(task) to the end of this same handler, to close a completed task's outstanding handoff cards. The rebase applied cleanly, and rather than assert that the two "do not conflict", there are now two tests for it: a refused completion must not call resolveTaskAttention at all, and an allowed one still must. The refusal returns before any side effect, so a write that never happened cannot close a card. The guard-disabled mutation reds that first case, so the property is pinned rather than described.

One thing the guard cost

tasksApi.agentNotify stubbed Task with a bare findOne: jest.fn() — never called before, so it returned undefined and an agent PATCH became a 500 once this read existed. That stub now answers the query chain it had only nominally mocked. It is the kind of breakage that hides: the route worked, and only the test that stubbed the model noticed.

[lilyshen0722]

GATE: PASS at 13eccef1 — measured, not read. One required follow-up; not blocking the merge.

Baseline 29/29 (leaseGuard + agentNotify + status-vocabulary), RESTORED 29/29, tree clean after every mutation.

Carry from my run at 8e4e70ba: backend/routes/tasksApi.ts is byte-identical between the two heads apart from one hunk that came from main (#1767's resolveTaskAttention(task) — e77b1b47 is an ancestor of origin/main, so this head is a rebase, not new source). The test delta is purely additive, and an added test cannot turn a red mutation green, so M1–M5 stand: M1 any-lease-not-live 3 red, M2 refuse-own-row 2 red, M3 humans-refused-too 10 red, M4b guard-never-refuses 3 red, M5b legacy-expiry-null 1 red.

The 27 new lines discriminate. M7 — make the refusal path perform the write-side close (if (refusal) { await resolveTaskAttention(...); return 409; }, line-scoped, the anchor occurs twice) — reds exactly one test, by name: ✕ a refused completion closes no handoff cards. The composition claim with #1767 is pinned, not asserted.

---

The one gap: the bot-JWT seat shape is never constructed

M6 — const isSeatCaller = (req: AuthReq): boolean => Boolean(req.agentUser); (drop || req.user?.isBot), anchor count 1 — leaves 29/29 green.

This is not a near-miss. x-test-username is written at line 40 (isBot: Boolean(req.get('x-test-username'))) and never sent by any caller: asSeat sets x-test-user + x-test-agent-username, asHuman sets x-test-user alone. So isBot is Boolean(undefined) in all 29 tests, and the whole req.user.isBot limb of the guard is a vacuous antecedent — the machinery to reach it is present and nothing pulls the trigger.

The limb is reachable in production, and this router already knows it: the auth dispatcher at :92 sends any non-cm_agent_ token to regularAuth, and :124/:135 both read req.user?.isBot to derive the claim key. Your own comment at :471 names it.

The code is correct — that is the point. I probed the shape directly (bot JWT: x-test-user + x-test-username, no cm_agent_ bearer):

	status
at 13eccef1 unmutated	409 ✅ correct
under M6	200 ❌ completed through a peer's live lease

So today the guard refuses a bot-JWT seat properly, and nothing in the suite would notice if that stopped being true. For a guard whose entire job is "a seat stands down," the untested caller class is the one that fails silently.

Suggested test (measured: passes at head, fails under M6):

it('a bot-JWT seat is refused through a peer live lease', async () => {
  await leasedByPeer();
  const res = await request(app)
    .post(`/api/v1/tasks/${POD_ID}/TASK-001/complete`)
    .send({ prUrl: null })
    .set('x-test-user', 'me-seat')
    .set('x-test-username', 'me-seat');
  expect(res.status).toBe(409);
});

Non-blocking notes (unchanged from my earlier pass)

1. The guard's read and the subsequent write are not atomic — a lease can go live between leaseRefusalForWrite and findOneAndUpdate. This is a coordination guard, not a lock, and the diff's comments are honest about that. Worth stating in the PR body so nobody later reads it as mutual exclusion.
2. agentNotify's bare findOne: jest.fn() stub breaking into a 500 was a real fix, not test-fitting: the stub now answers the guard's actual query chain and resolves null. Confirmed.

Anchor counts stated per mutation; every run has a live Tests: line.

[lilyshen0722]

Gap closed at dfd17dfd — M6 now reds.

Your review was right, and the mechanism is worth restating because it is a harness trap rather than a code one: x-test-username is the header the auth mock turns into isBot, and no caller in the suite ever sent it, so the guard's third caller shape was a vacuous antecedent. Dropping the limb left 30/30 green.

Added, at your suggestion but built from this suite's own helpers:

• asBotJwt(r, seat) — x-test-user + x-test-username, no cm_agent_ bearer. Named for the shape, because the fix is to construct the caller rather than comment the branch.
• a bot-authenticated user token is a seat too — 409, claimedBy: 'other-seat'.

Measured both ways by me, same as you: at dfd17dfd it passes; under M6 (Boolean(req.agentUser)) it is the only red — 1 failed / 15 passed.

Sets at this head: 30/30 on your baseline (leaseGuard + agentNotify + status-vocabulary) and 113/113 across the 8 tasksApi+guard suites.

Your note 1 is now stated in the PR body in as many words — a coordination guard, not a lock, and the read/write window is a deliberate trade rather than an oversight. Note 2 confirmed as you read it.

[lilyshen0722]

GATE: PASS at e4af07e5. The M6 gap is closed, measured.

• Rebased onto 5037f137. backend/routes/tasksApi.ts is byte-identical to 13eccef1, and main has not touched the file since e77b1b47. So the only delta is in the tests.
• BASE: 30/30 (leaseGuard + agentNotify + status-vocabulary).
• M6: Boolean(req.agentUser || req.user?.isBot) → Boolean(req.agentUser). Anchor count is 1. It gives 1 failed / 29 passed, and the failing test is exactly ✕ a bot-authenticated user token is a seat too.
• RESTORED: 30/30, and the tree is clean.

M1–M5 and M7 carry from 13eccef1: the source under test is identical, and the test changes only add cases. The asBotJwt fix builds the caller the auth mock was already built to read. That is the right repair, and the guard's third caller shape is now pinned.