Skip to content

fix(example-todo): make task completion possible for a normal user — stamp completed_date in the hook instead of demanding it from the caller (#7036) - #7222

Merged
os-help merged 5 commits into
mainfrom
claude/issue-7036-completed-date-stamp
Aug 10, 2026
Merged

fix(example-todo): make task completion possible for a normal user — stamp completed_date in the hook instead of demanding it from the caller (#7036)#7222
os-help merged 5 commits into
mainfrom
claude/issue-7036-completed-date-stamp

Conversation

@os-help

@os-help os-help commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Fixes #7036

A normal user could never mark a task complete. completed_date is readonly, so a non-system caller's write to it is stripped from the payload on the update path; completed_date_required then saw a blank value and refused the whole write. The two declarations were mutually unsatisfiable, and the app's own completeTask action — which sent exactly that pair — always failed.

Premise re-verified on current origin/main

The card's measured table reproduces exactly, against the app's real Task object on a real kernel (ObjectQL + sqlite-wasm), one row inserted as status: 'not_started':

A status+completed_date (user): REJECTED -> Completed date is required when status is Completed
A status only           (user): REJECTED -> Completed date is required when status is Completed
A status+completed_date  (sys): OK

The engine ordering the card cites is unchanged: reportDroppedFields(preRo, ..., 'readonly') still sits immediately above the evaluateValidationRules(updateSchema, ...) call in packages/objectql/src/engine.ts.

The ordering question the card left open, measured

Whether a beforeUpdate hook's writes land before or after the readonly strip was flagged UNVERIFIED. Measured answer, and it is the reason this shape works:

The hook runs BEFORE the strip, and the strip is built to spare it. triggerHooks('beforeUpdate', …) dispatches at engine.ts:7054; stripReadonlyFields runs at 7349. But the strip only deletes a key that both (a) the caller supplied and (b) still holds the caller's own value — the suppliedValues snapshot plus the Object.is identity check from #5591. A value a hook wrote is a platform value and survives. The engine's own comment says so in as many words: "Runs AFTER hooks/middleware stamped their columns".

Measured on the app's real object: a one-key user-context update ( { status: 'completed' } ) with the hook bound → OK, completed_date persisted.

Why not "readonly lifted"

Measured too, and it is strictly weaker — it fixes only the two-key call:

C status+completed_date (user): OK
C status only           (user): REJECTED -> Completed date is required when status is Completed

A status-only transition is the path a form, a list-view inline edit, and #6882's own flow test all drive, and lifting readonly leaves it broken while also handing a server-owned timestamp to the caller. So this PR keeps readonly and puts the stamp where the platform puts it.

What changed

  • src/objects/task.hook.ts — a beforeUpdate leg stamps completed_date on the transition into completed and clears it on the transition out. The stamp is unconditional, not ??=: if a caller supplied the key and the hook left it alone, the value would still be the caller's, the strip would delete it, and the rule would refuse the write again.
  • Registered the hook. task_logic was never in defineStack({ hooks }), so the entire file was dead metadata — it type-checked, it read as wired, and it never ran. collectBundleHooks walks that array and nothing else. Both sibling example apps (app-crm, app-showcase) already register hooks: allHooks; this brings app-todo in line.
  • Repaired the two pre-existing legs. They read the record off ctx.input (ctx.input.priority, ctx.input.status) rather than ctx.input.data. Per the HookContextSchema.input contract table, input is an envelope — { data, options } on insert, { id, data, options } on update — so those assignments set keys nothing reads. This was invisible while the hook was unregistered and would have shipped live otherwise.
  • src/actions/task.handlers.tscompleteTask and massCompleteTasks send status alone.
  • src/objects/task.object.ts — comments only; completed_date stays readonly and the validation rule stays. The rule is now the assertion that the stamp happened: if the hook is ever unregistered or its guard breaks, the write is refused loudly instead of committing a completed task with no completion date.

Leaving completed clears the stamp

Documented in the object and hook metadata, and tested. completed_date means "when this task was completed", so a reopened task must not carry one — a retained stamp is a stale timestamp every report and list view reads as fact. Guarded on status actually being part of the write, so an unrelated edit of a completed task ( { progress_percent: 100 } ) is not read as a reopen.

Tests

test/task-completion-trigger.test.ts — this card owns it. #6882's deliberate CREATE-seed workaround is gone; the task is created as a user creates one and the completion is a real user-context update. Six cases added: the headline case, the caller-echoes-the-key case (#5591 direction), the clear-on-reopen case, the forge-outside-a-transition case (#2948 unchanged), the wiring pin, and a reverse case.

Reverse verification — predicted red, and red for the right reason. Restoring the pre-fix hook turns 5 of 11 cases red, four of them raising ValidationError: Completed date is required when status is Completed — the defect's own message. The in-suite reverse case rebuilds that by withholding exactly one thing (the hook binding) from an otherwise identical kernel, and asserts the refusal rather than a missing value: an unstamped completion does not commit quietly, it is refused.

Tests  89 passed (89)          # examples/app-todo, full suite
tsc --noEmit                   # exit 0
eslint (5 changed files)       # exit 0
node scripts/check-nul-bytes.mjs  # OK, no raw ASCII control bytes

test/seed-check.ts boots the real config through AppPlugin and confirms the registration is live rather than merely declared:

INFO [AppPlugin] Bound declarative hooks {"appId":"com.example.todo","hookCount":1,"functionCount":0}
Found Tasks: 8

Changeset

None. examples/** releases nothing, and pr-automation.yml names it explicitly among the paths whose route is the skip-changeset label rather than a changeset file. Label applied.

Declared-region deviation — one file, disclosed

My dispatch region was src/objects/task.object.ts, task.hook.ts, src/actions/task.handlers.ts and this test file, with the app's defineStack registration site barred as sibling #7037's ground. This PR adds two lines to objectstack.config.ts (an import and hooks: [taskHook]).

The region was declared before anyone knew the hook was unregistered — that fact came out of the measurement above. Without those two lines the hook never runs and the PR fixes nothing, and every alternative measured is either strictly weaker (readonly lifted) or teaches a worse pattern in a reference app (registering a plain object hook imperatively from the action-handler bootstrap). The addition is placed next to data:, well away from where #7037's functions: key lands, and origin/main was merged before opening this PR. src/flows/task.flow.ts and test/task-recurrence.test.ts are untouched. Flagging it for the reviewer rather than deciding it silently.


Generated by Claude Code

claude added 2 commits August 10, 2026 02:58
…7036)

`completed_date` is `readonly` (a non-system caller's write is stripped on the
update path) and `completed_date_required` then refused the write for missing
exactly the value that had just been dropped, so `completeTask` — and every
user-driven status transition into `completed` — always failed.

Fixed in the metadata, the platform's system-stamp shape: `task.hook.ts` gains
a `beforeUpdate` leg that stamps `completed_date` on the transition into
`completed` and clears it on the transition out. A hook's write survives the
readonly strip by design (#2948/#5591), so the rule is satisfied by the server
and callers send `status` alone.

Also registers the hook in `defineStack({ hooks })` — it had never been in that
array, so the whole file was dead metadata — and repairs the two existing legs,
which read the record off `ctx.input` instead of `ctx.input.data`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 10, 2026 3:46am

Request Review

@os-help os-help added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 10, 2026 — with Claude
@github-actions github-actions Bot added the tests label Aug 10, 2026
claude added 2 commits August 10, 2026 03:20
… not `console`

The root type program has `lib: ["ES2020"]` and no node types, so `console` is
undefined there. The two calls were pre-existing but had never been measured:
`task.hook.ts` entered the root program for the first time in this branch,
because until now nothing imported it. Routing them through the engine handle
the hook context already carries is also the better shape for a reference app —
it honours the kernel's configured log level.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy
#7036)

`@objectstack/example-todo` is a changeset-versioned package (it carries a
version and a CHANGELOG, and `.changeset/config.json` does not ignore it), and
the sister repair #6882 shipped with one. This change is user-visible —
`completeTask` went from always failing to working — so it declares a patch
rather than taking the `skip-changeset` route.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy
@github-actions github-actions Bot added documentation Improvements or additions to documentation tooling labels Aug 10, 2026
@os-help os-help removed documentation Improvements or additions to documentation tooling skip-changeset PR has no user-facing published change; bypasses the changeset gate labels Aug 10, 2026 — with Claude
@os-help
os-help marked this pull request as ready for review August 10, 2026 04:08
@os-help
os-help added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 3d89777 Aug 10, 2026
34 checks passed
@os-help
os-help deleted the claude/issue-7036-completed-date-stamp branch August 10, 2026 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

2 participants