Repository navigation
Task lifecycle hook — completed_at and last_update_at stamping #3
Description
Activity
Claim: PM loop round 1
Session:session_01SqkTcrxUFci7nqXdbBSe2p
Branch:claude/issue-3-task-hook
Worktree:duly-issue-3
Domain: n/a (single-package app repo; nodomain:*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: opus
Clause-②: 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
{ "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-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
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:
- last_update_at does not advance when an attachment is added to a task #28 (attachments) — correct call. The clause in the issue was mine and it was wrong: I wrote "or an attachment" without checking that attachments are
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. - Reopening a done task fails to clear completed_at when the caller also sends completed_at: null #31 — an upstream
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. - A task cannot be created directly in
done— decide whether that is the intent #32 — real product question, queued.
Merging.
Generated by Claude Code
- last_update_at does not advance when an attachment is added to a task #28 (attachments) — correct call. The clause in the issue was mine and it was wrong: I wrote "or an attachment" without checking that attachments are
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.tsthat 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_atandlast_update_atare bothreadonly: true, meaning a non-system caller's write is stripped. The hook is the one writer.beforeInsert
last_update_at = nowbeforeUpdate
status = 'done'→ stampcompleted_at = nowdone→ clearcompleted_atto nulllast_update_at = nowonly when something meaningful changed:status,note,skip_reason, or an attachment. See below.The trap
last_update_atis the stagnation signal — the "Not moving" view isstatus 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_taskhas a validation rulecompleted_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_atsetdone→in_progress) clearscompleted_at; the record then passes validationcompleted_atexplicitly has it stripped and replacednoteadvanceslast_update_atbusiness_unit(or any bulk administrative write) does not advancelast_update_at— assert this directly, it is the whole pointGates
pnpm validate && pnpm typecheck && pnpm test && pnpm build.