Repository navigation
Task lifecycle hook — completed_at and last_update_at stamping #3
Description
Two server-owned timestamps on duly_task. One of them is the most useful number in the product.
Files you own
src/hooks/task.hook.ts(new), added todulyHooksinsrc/hooks/index.tstest/task-hook.test.ts(new)
Hooks are read from defineStack({ hooks }) only — a *.hook.ts that is not in the barrel type-checks, reads as wired, and never runs. The barrel is already imported by the config; do not touch the config.
Behaviour
completed_at and last_update_at are both readonly: true, meaning a non-system caller's write is stripped. The hook is the one writer.
beforeInsert
last_update_at = now
beforeUpdate
- transition into
status = 'done'→ stampcompleted_at = now - transition out of
done→ clearcompleted_atto null last_update_at = nowonly when something meaningful changed:status,note,skip_reason, or an attachment. See below.
The trap
last_update_at is the stagnation signal — the "Not moving" view is status in (open, in_progress) AND last_update_at < {14_days_ago}. If the hook stamps on every update, then any unrelated write — a bulk re-owner, a business-unit backfill, an import — silently resets the stagnation clock on the entire table and the signal goes quiet exactly when it matters.
Stamp on the fields a human touching the task would change. Do not stamp on system-owned or administrative field changes.
The other trap
duly_task has a validation rule completed_at_required_when_done. It is satisfied by the server: this hook stamps before validation runs, so a completion write carrying only { status: 'done' } passes. That rule exists as the assertion that the stamp happened — if this hook is ever unregistered, the write is refused loudly instead of committing a done task with no timestamp. Do not "fix" the rule; make the hook satisfy it.
Acceptance
{ status: 'done' }alone commits, withcompleted_atset- reopening (
done→in_progress) clearscompleted_at; the record then passes validation - a caller passing
completed_atexplicitly has it stripped and replaced - editing
noteadvanceslast_update_at - changing only
business_unit(or any bulk administrative write) does not advancelast_update_at— assert this directly, it is the whole point - re-saving with no changes does not advance it either
Gates
pnpm validate && pnpm typecheck && pnpm test && pnpm build.
Activity
os-warren commentedon Sep 1, 2026
Claim: PM loop round 1
Session:
session_01SqkTcrxUFci7nqXdbBSe2pBranch:
claude/issue-3-task-hookWorktree:
duly-issue-3Domain: n/a (single-package app repo; no
domain:*lanes in this repo)File surface:
src/hooks/,test/task-hook.test.ts(stop on breach; explain in the report)Container & model: M,
mode:subagent,model: opusClause-②: no — application metadata only; no protocol contract surface touched.
Serial constraints cleared: batch 1 is #1 / #3 / #5 / #6, file surfaces disjoint.
src/objects/task.object.tsis READ-ONLY for this task — itscompleted_at_required_when_donevalidation is the assertion this hook must satisfy, not a rule to relax.Generated by Claude Code
os-warren commentedon Sep 1, 2026
{ "issue": 3, "status": "done", "branch": "claude/issue-3-task-hook", "pr": "https://github.com/objectstack-ai/duly/pull/35", "premise_still_valid": true, "summary": "Added src/hooks/task.hook.ts (registered in dulyHooks) stamping completed_at on the transition into/out of done and last_update_at only when status, note or skip_reason actually changed against ctx.previous; src/objects/task.object.ts untouched. All three PM mechanism assumptions were verified empirically and all three held. Two deviations to flag: the issue's 'or an attachment' clause is NOT implemented (attachments are sys_attachment rows, so no duly_task write occurs at all — filed as #28), and the hook is deliberately written as one self-contained function so objectstack build lowers it to a metadata body instead of the legacy bundled-runtime fallback, which only warns. While testing I found the suite was silently reading dist/objectstack.json instead of src/ — it passed with the barrel entry deleted until that was closed.", "tests": "All four gates green at 864ba78. `pnpm validate` -> '✓ Validation passed (237ms)'. `pnpm typecheck` -> exit 0, no diagnostics. `pnpm test` -> 'Test Files 2 passed (2) / Tests 27 passed (27)'. `pnpm build` -> '✓ Build complete', 'Artifact: dist/objectstack.json (53.3 KB)', no lowering warning. Exit codes captured by redirecting to a file before reading, never through a pipe. test/task-hook.test.ts boots a REAL ObjectQL engine (in-memory driver) via createStandaloneStack + AppPlugin over the app's own objectstack.config.ts, so the handler runs only because AppPlugin found it in the real config. HERMETICITY FIX: left at its default createStandaloneStack resolves dist/objectstack.json under cwd and the kernel loads objects AND hooks from it; with a local build present the suite reported on the last build rather than on src/ (log line '[MetadataPlugin] Loading metadata from local artifact file'), and the registration ablation came back GREEN. Closed with an artifactPath sentinel; the final run logs 'no compiled artifact yet' even though dist/objectstack.json exists on disk. TWO ABLATIONS, each mutation confirmed on disk by grep counts of the injected and the removed text, each restored via a trap and the restore confirmed by a zero marker count plus an empty git diff: (A) stamp last_update_at unconditionally -> 5 failed, including 'does NOT advance on an administrative write (business_unit)', 're-owner', 're-date', 're-save with no changes', 'does NOT accept a caller-supplied value'; re-run on the final hermetic tree with the same result. (B) remove the entry from dulyHooks -> 11 failed: both registration tests and every positive behaviour test. No rebuild was needed between legs — vitest imports the TS source directly, and the artifact is now excluded by construction. SEPARATELY MEASURED (not in the suite, since pnpm test runs before pnpm build): the lowered body from dist/objectstack.json was executed against the real QuickJS sandbox via hookBodyRunnerFactory + QuickJSScriptRunner, confirming new Date().toISOString() works there and both stamps land — that is the surface a served artifact runs.", "open_questions": [], "out_of_scope_findings": [ "filed as #28: last_update_at does not advance when an attachment is added — the issue's 'or an attachment' clause. Attachments are sys_attachment rows carrying parent_object/parent_id, so adding one is an insert on THAT object and no duly_task write happens; it needs a second hook on sys_attachment plus an isSystem write-back to get past the readonly strip, and answers to product questions #3 did not settle.", "filed as #31: reopening a done task fails to clear completed_at when the caller ALSO sends completed_at: null. Upstream objectql limitation — stripReadonlyFields uses Object.is(result[name], supplied[name]) to tell a hook-derived value from a caller value, and cannot distinguish 'the hook deliberately wrote the same value' from 'the hook never touched it'. Measured: persisted completed_at stayed at the old timestamp on an in_progress record. Deliberately NOT worked around in the hook (that would be a consumer-side hack around a producer defect); the common paths are unaffected and covered by passing tests.", "filed as #32: a task cannot be created directly in status 'done' — completed_at is readonly and beforeInsert stamps only last_update_at (as #3 specifies), so completed_at_required_when_done refuses the insert for every non-isSystem caller. Pinned in the suite as the negative control proving that rule is live. May be correct by design; needs a product decision rather than a guess." ] }Generated by Claude Code
os-warren commentedon Sep 1, 2026
os-dev-report
Marker note: the previous comment carried this report behind an HTML-comment marker, which GitHub's body sanitizer stripped on save — a scan keying on that marker would not have seen it. This comment repeats the report with the marker as literal first-line text, which survives. Treat this one as the report of record.
{ "issue": 3, "status": "done", "branch": "claude/issue-3-task-hook", "pr": "https://github.com/objectstack-ai/duly/pull/35", "premise_still_valid": true, "summary": "Added src/hooks/task.hook.ts (registered in dulyHooks) stamping completed_at on the transition into/out of done and last_update_at only when status, note or skip_reason actually changed against ctx.previous; src/objects/task.object.ts untouched. All three PM mechanism assumptions were verified empirically and all three held. Two deviations to flag: the issue's 'or an attachment' clause is NOT implemented (attachments are sys_attachment rows, so no duly_task write occurs at all - filed as #28), and the hook is deliberately written as one self-contained function so objectstack build lowers it to a metadata body instead of the legacy bundled-runtime fallback, which only warns. While testing I found the suite was silently reading dist/objectstack.json instead of src/ - it passed with the barrel entry deleted until that was closed.", "tests": "All four gates green at 864ba78. `pnpm validate` -> 'Validation passed (237ms)'. `pnpm typecheck` -> exit 0, no diagnostics. `pnpm test` -> 'Test Files 2 passed (2) / Tests 27 passed (27)'. `pnpm build` -> 'Build complete', 'Artifact: dist/objectstack.json (53.3 KB)', no lowering warning. Exit codes captured by redirecting to a file before reading, never through a pipe. test/task-hook.test.ts boots a REAL ObjectQL engine (in-memory driver) via createStandaloneStack + AppPlugin over the app's own objectstack.config.ts, so the handler runs only because AppPlugin found it in the real config. HERMETICITY FIX: left at its default createStandaloneStack resolves dist/objectstack.json under cwd and the kernel loads objects AND hooks from it; with a local build present the suite reported on the last build rather than on src/ (log line '[MetadataPlugin] Loading metadata from local artifact file'), and the registration ablation came back GREEN. Closed with an artifactPath sentinel; the final run logs 'no compiled artifact yet' even though dist/objectstack.json exists on disk. TWO ABLATIONS, each mutation confirmed on disk by grep counts of the injected and the removed text, each restored via a trap and the restore confirmed by a zero marker count plus an empty git diff: (A) stamp last_update_at unconditionally -> 5 failed, including 'does NOT advance on an administrative write (business_unit)', 're-owner', 're-date', 're-save with no changes', 'does NOT accept a caller-supplied value'; re-run on the final hermetic tree with the same result. (B) remove the entry from dulyHooks -> 11 failed: both registration tests and every positive behaviour test. No rebuild was needed between legs - vitest imports the TS source directly, and the artifact is now excluded by construction. SEPARATELY MEASURED (not in the suite, since pnpm test runs before pnpm build): the lowered body from dist/objectstack.json was executed against the real QuickJS sandbox via hookBodyRunnerFactory + QuickJSScriptRunner, confirming new Date().toISOString() works there and both stamps land - that is the surface a served artifact runs.", "open_questions": [], "out_of_scope_findings": [ "filed as #28: last_update_at does not advance when an attachment is added - the issue's 'or an attachment' clause. Attachments are sys_attachment rows carrying parent_object/parent_id, so adding one is an insert on THAT object and no duly_task write happens; it needs a second hook on sys_attachment plus an isSystem write-back to get past the readonly strip, and answers to product questions #3 did not settle.", "filed as #31: reopening a done task fails to clear completed_at when the caller ALSO sends completed_at: null. Upstream objectql limitation - stripReadonlyFields uses Object.is(result[name], supplied[name]) to tell a hook-derived value from a caller value, and cannot distinguish 'the hook deliberately wrote the same value' from 'the hook never touched it'. Measured: persisted completed_at stayed at the old timestamp on an in_progress record. Deliberately NOT worked around in the hook (that would be a consumer-side hack around a producer defect); the common paths are unaffected and covered by passing tests.", "filed as #32: a task cannot be created directly in status 'done' - completed_at is readonly and beforeInsert stamps only last_update_at (as #3 specifies), so completed_at_required_when_done refuses the insert for every non-isSystem caller. Pinned in the suite as the negative control proving that rule is live. May be correct by design; needs a product decision rather than a guess." ] }Generated by Claude Code
os-warren commentedon Sep 1, 2026
ACCEPT — PM review of #35, round 1.
Checklist: draft, base
main,Fixes #3first line, 3 files all inside the declared surface,src/objects/task.object.tsandobjectstack.config.tsuntouched, no NUL bytes, no changeset (correct here). CIverifysuccesson864ba78.Verified independently. Branch fetched,
origin/mainmerged in (so this was measured against the landing state, with #1's period engine present), all four gates re-run:EXIT=0each, 116 tests.Then the one ablation that had to be mine rather than yours: with
dist/objectstack.jsonon disk (54,573 bytes), I emptieddulyHooksand re-ran. Red — 11 failures including both registration tests. That confirms theartifactPathsentinel actually closed the hole, and it is the check everything else depended on: if the suite had still been reading the artifact, every ablation in this PR — including the one guarding the deliverable — would have been reporting on the last build instead of onsrc/, and would have looked exactly as green.Finding that yourself, in your own harness, and closing it before reporting is the most valuable thing in this PR. A suite that passes with the metadata deleted is worse than no suite, because it is trusted.
The guard is the right shape: pre-image comparison over exactly
status/note/skip_reason, fourdoes NOT advanceassertions, and a no-pre-image context stamping nothing — which is precisely the unscoped bulk write the stagnation signal most needs to survive.On the three findings:
sys_attachmentinserts that never touchduly_task. There was no way to implement it inside this card's surface, and the product questions it raises (does removing one count? an importer's?) genuinely were not settled. Reporting beat guessing.Object.islimitation in the readonly strip. Agreed with not working around it: a consumer-side hack around a producer defect is how a workaround becomes permanent. The common paths are covered.done— decide whether that is the intent #32 — real product question, queued.Merging.
Generated by Claude Code