Uh oh!
There was an error while loading. Please reload this page.
Task lifecycle hook — completed_at and last_update_at stamping - #35
Merged
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p
Left at its default, createStandaloneStack resolves <cwd>/dist/objectstack.json and the kernel loads its metadata — objects AND hooks — from that file. With a local pnpm build present the suite then reported on the last build rather than on src/, and the registration ablation came back green with the barrel entry deleted. Point the lookup at a sentinel path instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p
os-warren
marked this pull request as ready for review
September 1, 2026 04:04
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#3
Two server-owned timestamps on
duly_task, written by one hook registered indulyHooks.What it does
beforeInsert— stampslast_update_at. A brand-new task has just been touched, by definition.beforeUpdatedone→completed_at = nowdone→completed_at = nulllast_update_at = nowonly whenstatus,noteorskip_reasonactually changed against the pre-imageEverything keys off
ctx.previous, so these are transitions rather than states. That is what stops a whole-record form re-submittingstatus: 'done'from overwriting the original completion instant on every save, and what makes "re-saving with no changes" a no-op.The stagnation signal
last_update_atfeeds the "Not moving" view —status in (open, in_progress) AND last_update_at < {14_days_ago}. A hook that stamped on every update would let one bulk re-owner, a business-unit backfill or an import silently reset the clock across the whole table. Nothing errors; the numbers just quietly improve and the signal goes quiet exactly when it matters.So the touch list is exactly
status,note,skip_reason.owner,business_unit,assignment,duty,due_date,visible_from,period_key,sourceandsubjectare deliberately excluded, and three of them are asserted directly:A context with no pre-image — the whole-operation dispatch of an unscoped predicate write, i.e. the bulk write this hook most needs to survive — stamps nothing at all.
Three platform assumptions, verified rather than assumed
All three held. Measured against a real booted engine, not read off the docs:
beforeUpdatecan see the previous field valueshookContext.previousfrom a pre-read of the row before dispatching thebeforeUpdatechain, and throwsrecordNotFoundErrorif that read comes back empty — so on the by-id path it is always present. Pinned by a probe hook in the suite.beforeUpdatestamp lands ahead of validationbeforeUpdate→validateRecord→ readonly strip →evaluateValidationRules. A write carrying only{ status: 'done' }therefore commits.readonlystripping blocks caller values while the hook's write passesTests
test/task-hook.test.tsboots a real ObjectQL engine (in-memory driver) with this app's ownobjectstack.config.tsas the bundle. The handler runs only becauseAppPluginfound it in the real config, so the suite cannot pass on dead metadata.Two things worth flagging to a reviewer:
The suite is pinned away from the build artifact. Left at its default,
createStandaloneStackresolvesdist/objectstack.jsonunder the working directory, and when a localpnpm buildhas left one there the kernel loads its metadata — objects and hooks — from that file instead of from the config. The suite then reports on the last build rather than onsrc/, and behaves differently in CI (wherepnpm testruns beforepnpm build) than on a developer's machine. This is not hypothetical: it is what made the registration ablation come back green. TheartifactPathsentinel inbeforeAllis what closes it.A negative control guards the completion assertions.
completed_at_required_when_doneis asserted to still refuse an insert of adonetask, so if that rule ever goes quiet the completion tests stop proving anything.Both ablations were run to confirm the assertions can fail:
last_update_atunconditionally (the classic wrong hook) → 5 failed, including thebusiness_unitassertiondulyHooks→ 11 failed, the registration and every positive behaviour testNot in this PR
The attachment clause is not implemented — filed as #28. #3 asks for
last_update_atto advance on "an attachment", but attachments aresys_attachmentrows carryingparent_object/parent_id, so adding one is an insert on that object and noduly_taskwrite happens at all. It needs a second hook onsys_attachment, anisSystemwrite-back to get past the readonly strip, and answers to product questions #3 did not settle (does removing an attachment count? does an importer's?).Also filed: #31 (reopening fails to clear
completed_atwhen the caller also sendscompleted_at: null— an upstreamObject.islimitation in the readonly strip, deliberately not worked around here) and #32 (a task cannot be created directly indone— needs a product decision).Gates
All four green at
864ba78:The handler is written as one self-contained function so
objectstack buildlowers it to a metadatabodyrather than falling back to the legacy bundled runtime module — a fallback that only warns. The lowered body was executed against the real QuickJS sandbox to confirm it behaves identically there, since that is the surface a served artifact runs.src/objects/task.object.tswas not touched.Generated by Claude Code