fix(tasks): defensive user fallback in handleCancel persist (#1108) #1110

Merged

reviewer merged 1 commit from code-lead/1108 into main

2026-05-11 19:01:43 +00:00

code-lead commented

2026-05-11 18:53:20 +00:00

Collaborator

Closes #1108.

Summary

handleCancel populates task_history.user from the in-memory record's user, then falls back through the running task's git_author to a sentinel (<unknown>) so the NOT NULL invariant on task_history.user is always honoured.
Both inline persistTask call sites — the queued-drop and the running-cancel — route through a new persistCancelOrLog helper that wraps the synchronous SQLite call in try/catch so any residual failure (DB locked, schema drift, etc.) logs a structured [cancel] line instead of escaping as unhandledRejection.
The previous crash was reproducible on cursor-cli sessions where dispatch never populated request.git_author; the in-memory TaskRecord.user was undefined, drizzle serialised it as NULL, and SQLite rejected the insert with SQLITE_CONSTRAINT_NOTNULL.

Test plan

bun test apps/server/src/main.test.ts -t "POST /cancel" — all 16 cancel-route tests pass (4 new + 12 existing).
New tests under task_history persistence (#1108):
- running-cancel persists with the record's user.
- running-cancel with empty record.user falls back to request.git_author; no unhandledRejection fires.
- running-cancel with no user anywhere falls back to CANCEL_UNKNOWN_USER.
- queued-drop persists with the dropped task's user.
Full bun test apps/server/src/main.test.ts (74 pass).
just typecheck clean.
just lint / just fmt-check clean on touched files.

Once #task-history-on-start lands, the row will already exist by cancel time and this handler can switch to UPDATE instead of INSERT — at which point the sentinel fallback can be removed.
The same shared helper (apps/server/src/domain/dispatch/cancel.ts::persistAndBroadcastCancellation) carries the identical user shape and has the same latent vulnerability via the issues.unassigned webhook path; a follow-up should fold that helper through the same defensive pattern. Out of scope here per the AC.

🤖 Generated with Claude Code

Closes #1108. ## Summary - `handleCancel` populates `task_history.user` from the in-memory record's `user`, then falls back through the running task's `git_author` to a sentinel (`<unknown>`) so the `NOT NULL` invariant on `task_history.user` is always honoured. - Both inline `persistTask` call sites — the queued-drop and the running-cancel — route through a new `persistCancelOrLog` helper that wraps the synchronous SQLite call in try/catch so any residual failure (DB locked, schema drift, etc.) logs a structured `[cancel]` line instead of escaping as `unhandledRejection`. - The previous crash was reproducible on cursor-cli sessions where dispatch never populated `request.git_author`; the in-memory `TaskRecord.user` was `undefined`, drizzle serialised it as NULL, and SQLite rejected the insert with `SQLITE_CONSTRAINT_NOTNULL`. ## Test plan - [x] `bun test apps/server/src/main.test.ts -t "POST /cancel"` — all 16 cancel-route tests pass (4 new + 12 existing). - [x] New tests under `task_history persistence (#1108)`: - running-cancel persists with the record's `user`. - running-cancel with empty `record.user` falls back to `request.git_author`; no `unhandledRejection` fires. - running-cancel with no user anywhere falls back to `CANCEL_UNKNOWN_USER`. - queued-drop persists with the dropped task's `user`. - [x] Full `bun test apps/server/src/main.test.ts` (74 pass). - [x] `just typecheck` clean. - [x] `just lint` / `just fmt-check` clean on touched files. ### Related (out of scope) - Once `#task-history-on-start` lands, the row will already exist by cancel time and this handler can switch to UPDATE instead of INSERT — at which point the sentinel fallback can be removed. - The same shared helper (`apps/server/src/domain/dispatch/cancel.ts::persistAndBroadcastCancellation`) carries the identical `user` shape and has the same latent vulnerability via the `issues.unassigned` webhook path; a follow-up should fold that helper through the same defensive pattern. Out of scope here per the AC. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 1 commit

2026-05-11 18:53:20 +00:00

fix(tasks): defensive user fallback in handleCancel persist (#1108 )

qa / sql-layer-check (pull_request) Successful in 40s

Details

qa / dockerfile (pull_request) Successful in 43s

Details

qa / db-schema (pull_request) Successful in 1m6s

Details

qa / i18n-string-check (pull_request) Successful in 1m26s

Details

qa / qa-1 (pull_request) Successful in 4m52s

Details

qa / qa (pull_request) Successful in 0s

Details

f1805ba880

The cancel-path `persistTask` previously passed `record.user` straight
through. When the in-memory `TaskRecord.user` was undefined (cursor-cli
sessions whose dispatch never populated `request.git_author`), the
drizzle insert hit `SQLITE_CONSTRAINT_NOTNULL` on `task_history.user`
and the error escaped the async dispatch chain as `unhandledRejection`,
killing the operator's cancel attempt mid-flight.

Honour the NOT NULL invariant by falling back through the running
task's git_author to an explicit `<unknown>` sentinel, and wrap the
persist call in try/catch so residual failures log instead of crashing.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead requested review from reviewer

2026-05-11 18:59:23 +00:00

reviewer approved these changes

2026-05-11 19:01:36 +00:00

reviewer left a comment

LGTM. Clean, targeted fix for the SQLITE_CONSTRAINT_NOTNULL crash on task_history.user.

Correctness

persistCancelOrLog correctly uses try/catch (not .catch()) since persistTask is a synchronous bun:sqlite call. The wrapper mirrors the existing non-awaited call pattern and logs residual failures rather than letting them surface as unhandledRejection.
The fallback chain record.user || request.git_author || CANCEL_UNKNOWN_USER is sound: || catches both undefined and "", which correctly handles the cursor-CLI dispatch path where git_author was never populated. The <unknown> sentinel satisfies the NOT NULL invariant as a last resort.
Both cancel-path call sites (queued-drop and running-cancel) are updated consistently.

Tests

All four cases are covered:

Happy path — record user is present and used directly.
Empty record.user regression — falls through to git_author; unhandledRejection listener verifies the crash path is gone.
Both fields blank — sentinel is written to task_history.user.
Queued-drop path — user resolved from the dropped task.

CI: ✅ green

No concerns.

LGTM. Clean, targeted fix for the `SQLITE_CONSTRAINT_NOTNULL` crash on `task_history.user`. **Correctness** - `persistCancelOrLog` correctly uses `try/catch` (not `.catch()`) since `persistTask` is a synchronous `bun:sqlite` call. The wrapper mirrors the existing non-awaited call pattern and logs residual failures rather than letting them surface as `unhandledRejection`. - The fallback chain `record.user || request.git_author || CANCEL_UNKNOWN_USER` is sound: `||` catches both `undefined` and `""`, which correctly handles the cursor-CLI dispatch path where `git_author` was never populated. The `<unknown>` sentinel satisfies the `NOT NULL` invariant as a last resort. - Both cancel-path call sites (queued-drop and running-cancel) are updated consistently. **Tests** All four cases are covered: 1. Happy path — record user is present and used directly. 2. Empty `record.user` regression — falls through to `git_author`; `unhandledRejection` listener verifies the crash path is gone. 3. Both fields blank — sentinel is written to `task_history.user`. 4. Queued-drop path — user resolved from the dropped task. **CI**: ✅ green No concerns.

reviewer merged commit 6c2c2afc48 into main

2026-05-11 19:01:43 +00:00