One-click completion with undo, and skip-with-reason

[os-warren]

The interaction the whole product rests on. If ticking a task costs more than a second, the list stops being maintained and every metric downstream becomes a report on a dataset nobody keeps.

Blocked-by: #3 (the hook does the stamping; these actions only flip status).

Files you own

• src/actions/task.actions.ts (new) — action metadata, added to dulyActions in src/actions/index.ts
• src/actions/task.handlers.ts (new)
• src/actions/register-handlers.ts — add your registerTaskActionHandlers(ql) call inside the existing function
• test/task-actions.test.ts (new)

Handlers are runtime code wired through register-handlers.ts, which the config already imports. An action whose handler is unregistered renders, is clickable, and fails at call time — there is no author-time gate for it.

Actions

duly_task_complete — row action, primary.

• Sets status = 'done'. That is the entire payload. completed_at is the hook's job (Task lifecycle hook — completed_at and last_update_at stamping #3); do not send it.
• No modal. No confirmation dialog. No required note, attachment or percentage.
• Visible when record.status is open or in_progress. CEL predicates reference record.<field> — a bare status evaluates to null and hides the action on every record, and pnpm validate now rejects it.

duly_task_undo — row action, appears on a just-completed row.

• Sets status = 'in_progress'; the hook clears completed_at.
• Undo replaces confirmation. This is the design: an accidental tick costs one click to reverse, so the tick itself needs no ceremony.

duly_task_skip — row action, secondary.

• Sets status = 'skipped' and requires skip_reason. This is the one place a modal is correct, because the object's skip_needs_reason validation will reject the write without it.
• Skipping is a legitimate outcome — "the plant was down, there was nothing to return". Forcing that answer to be recorded as done or left open corrupts the data either way.

Also expose complete and skip as bulk actions on the list views: a week's worth of ticks in one gesture is the difference between a Monday-morning habit and a chore.

Do not

• add a completion percentage, a required evidence upload, or a "are you sure?" step
• make attachments a condition of completing anything — duly_task.enable.files is on so people can attach, never so they must
• write completed_at or last_update_at from a handler

Acceptance

• completing sends { status: 'done' } and nothing else; the record comes back with completed_at set
• undo returns the record to in_progress with completed_at null
• skip without a reason is rejected by validation with the object's message
• bulk-completing 20 selected tasks issues one write path and leaves all 20 stamped
• action predicates are all record.<field>-qualified and pnpm validate passes

Gates

pnpm validate && pnpm typecheck && pnpm test && pnpm build.

[os-warren]

Claim: PM loop round 2
Session: session_01SqkTcrxUFci7nqXdbBSe2p
Branch: claude/issue-4-one-click-complete
Worktree: duly-issue-4
Domain: n/a (single-package app repo)
File surface: src/actions/task.actions.ts, src/actions/task.handlers.ts, src/actions/index.ts, src/actions/register-handlers.ts, src/views/task.view.ts (bulk actions only), test/task-actions.test.ts (stop on breach; explain in the report)
Container & model: M, mode:subagent, model: opus
Clause-②: no — application metadata; no protocol contract surface.
Serial constraints cleared: round 2 is #29 / #2 / #23 / #4, file surfaces disjoint. #5 landed at a4540ae and already added registerCatalogActionHandlers(ql) inside registerDulyActionHandlers — add your call beside it, do not restructure. #12 also touches src/views/task.view.ts and is deliberately not in this round; keep your edit there to the bulk-action wiring so the two do not collide across rounds.

---

Generated by Claude Code

[os-warren]

{
  "issue": 4,
  "status": "done",
  "branch": "claude/issue-4-one-click-complete",
  "pr": "https://github.com/objectstack-ai/duly/pull/41",
  "premise_still_valid": true,
  "summary": "Three row actions (duly_task_complete / duly_task_undo / duly_task_skip) plus bulk complete and bulk skip on the four multi-select task grids. ROUTE TAKEN, split: the platform DOES have a declarative field write for a SELECTION -- a list view's bulkActionDefs with operation:'update' and a static patch -- so both bulk forms are pure metadata, no action and no handler, and the write runs on the data plane under the caller's own permissions. It has NO row-level equivalent: ActionType is url|form|flow|script|api|modal, with no update_record type, no action effect and no patch key on ActionSchema, so the three row actions are type:'script' handlers. type:'api'+PATCH+bodyExtra was rejected as the declarative form because it is a declarative HTTP call, not a field write: it requires hand-writing /api/v1/data/duly_task/${ctx.recordId} into app metadata, nothing binds that string to objectName, nothing checks it at author time, and projectResolution:'required' registers only the environment-scoped route so the app would break on such a host with no diagnostic. type:'flow' + an update_record node was rejected as three flows to assign one string on the surface validate does not fully check (objectstack#14089). Gap filed upstream as objectstack#14092, with objectstack#14093 for the spec's own worked example of the type:'api' route, which omits the /data segment the shipped router mounts. Completing sends { status: 'done' } and nothing else; completed_at and last_update_at stay the #3 hook's. No confirmation, percentage or evidence gate on complete/undo -- undoable:true gives the platform toast Undo and duly_task_undo covers the mistake noticed after the toast. Skip collects a required reason, the one correct modal. Each handler re-checks availability and the transition server-side because ctx.engine is system-elevated and RLS-bypassing by design and visible is a UI hide, not authorization; the check keys on ctx.record.status rather than ctx.record.id because the dispatcher stamps the id on unconditionally after swallowing a failed subject read.",
  "tests": "All four gates green on f116421 (branch head, and the tree the union was run on -- git rev-parse --short HEAD = f116421). Every heavy command routed through scripts/pm/os-verify-lock.sh; verdicts read from its VERDICT line, not a bare $?. pnpm validate: '<check> Validation passed (332ms)', 5 Actions. pnpm typecheck: tsc --noEmit, silent, exit 0 inside the chained &&. pnpm test: 'Test Files 6 passed (6) / Tests 210 passed (210)' -- 35 of those are the new test/task-actions.test.ts. pnpm build: '<check> Build complete (498ms)', Artifact: dist/objectstack.json (73.0 KB). Wrapper line for the chained run: 'os-verify-lock: VERDICT command-exit 0 | held the lock 18s | waited 0s'. TWO ABLATIONS, both with trap '<restore>' EXIT INT TERM, both run from a COMMITTED tree so restore had a real recovery point, both restored and the clean tree verified afterwards with git status. No build/dist leg applies -- validate loads the config from source and vitest reads the TS directly -- but the mutation was proved on disk in each case before any result was read. (1) REGISTRATION TRAP: removed registerTaskActionHandlers(ql) from register-handlers.ts; on-disk proof was the call count going 1 -> 0 plus the mutated function body printed, with an explicit 'MUTATION DID NOT LAND ... reading aborted' exit-90 guard. Direction observed = the predicted one and it is the issue's central claim: VALIDATE_EXIT=0 with '<check> Validation passed (287ms)' -- an unregistered handler ships green -- while pnpm test went red with 17 failures headed by 'registration > every declared handler-backed action has a handler registered under duly_task' and 'registration > dispatches for real'. So the test is the only gate, which is why it dispatches through the engine's own executeAction rather than only inspecting the registry. (2) BARE PREDICATE: mutated duly_task_undo's visible to bare status == \"done\"; on-disk proof was qualified=0 / bare=1 plus all three visible: P lines printed, showing line 97 mutated and 68/125 untouched. Direction observed = rejection: VALIDATE_EXIT=1, \"action 'duly_task_undo' visible: bare reference `status` ... Write `record.status`. rule: expression-invalid\", and the suite caught it independently. Confirms action predicates ARE gated, unlike flow predicates (objectstack#14089). MEASURED FINDING pinned in the suite in both directions: a predicate (multi) update carries ONE payload for all N rows, so the completed_at the hook stamps for a genuinely-transitioning row lands on the whole batch -- bulk-completing a selection containing an already-done row moved that row's completion instant (before 04:37:26.560Z, after 04:37:26.571Z). The bulk defs' per-record visible predicate is what makes such a selection unreachable from the UI; filed as duly#39 for a server-side guard. Refusals are asserted by envelope (code + status), never by a bare toThrow: DULY_TASK_NOT_AVAILABLE/404, DULY_TASK_NO_SUBJECT/400, DULY_TASK_WRONG_STATUS/409, DULY_TASK_SKIP_NEEDS_REASON/400, and the object's own rule pinned separately as code VALIDATION_FAILED with message 'Say why the task was skipped.' so handler and rule cannot drift into one guard doing all the work. No control bytes: grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]' over all seven touched files came back clean. Branch merges cleanly onto the new origin/main (03d6afb, moved under me): git merge-tree --write-tree exited 0 with no conflict. CI not waited on, per the dispatch contract -- PM reads the real job conclusions.",
  "open_questions": [
    {
      "question": "Confirm the route read: I concluded the platform has NO declarative field-set for a ROW action and wrote three handlers, while taking the declarative form for bulk. The one thing that could overturn that is if you consider type:'api' + method:'PATCH' + bodyExtra to BE the declarative form the instruction meant. It is shipped as handlers; reversing it is a small, contained change to task.actions.ts (delete task.handlers.ts, no register-handlers entry).",
      "options": [
        "A - keep handlers for the rows (shipped). Real business need: the need is the tick itself, and this serves it with the write actually verified end-to-end against a booted engine. Long-term soundness: no transport detail pinned in app metadata, so the app survives a project-scoped host; contract-first, the gap is reported upstream rather than worked around. AI-safety: the failure mode (unregistered handler) is assertable and IS asserted, and the ablation proves the assertion is the gate; the authorization re-check is explicit rather than assumed. Startup discipline: costs ~200 lines of handler that exist only because the platform lacks the key.",
        "B - type:'api' PATCH at the data API, zero handler code. Real business need: same. Long-term soundness: WORSE -- hard-codes /api/v1/data/<object>/<id> into app metadata, which projectResolution:'required' does not serve, and duplicates the object name into a path literal nothing binds to objectName. AI-safety: WORST available -- no author-time check on the target, and nothing in this repo can verify it, so the acceptance criteria become unverified claims about a renderer that is not here; it parses green and 404s at the click, which is the same silent shape as the unregistered handler the issue warns about. Its one real advantage is genuine: the write would run under the caller's own permissions, so no hand-rolled authorization guard.",
        "C - type:'flow' + an update_record node. Long-term soundness: declarative and platform-native, no URL pinned. But it is one flow per button to assign one string plus a flow-run record per tick, it lands on src/flows/ which is outside this card's declared file surface and shared with #29 this round, and flow predicates are the one surface validate does not check (objectstack#14089)."
      ],
      "recommendation": "A, because the deciding axis is 'make AI-written metadata hard to get wrong', and it separates the options cleanly. Under A every acceptance criterion on the issue became a real assertion against a real write pipeline, and I proved by ablation that the assertion fails when the wiring is removed. Under B not one of them could be verified in this repo -- the correctness of the whole feature would rest on a hand-typed URL string with no gate at any layer, and the spec's own worked example of that string is wrong (objectstack#14093), which is fairly direct evidence about how easy it is to get wrong. B's authorization advantage is real and is exactly what I filed objectstack#14092 to ask for: the right fix is a declarative row-action field write that routes through the data plane, not an app hand-assembling the transport."
    }
  ],
  "out_of_scope_findings": [
    "filed as objectstack-ai/objectstack#14092: no declarative row-action field write, while bulkActionDefs has one -- the fourth platform gap from dogfooding duly, and the one this card was told to look for",
    "filed as objectstack-ai/objectstack#14093: ActionSchema.method's worked example points a type:'api' PATCH at /api/v1/sys_api_key/{id}, but the shipped router mounts /api/v1/data/:object/:id -- copying it 404s, with no gate at any layer",
    "filed as duly#39: a bulk status write re-stamps completed_at on an already-done row in the same batch (measured); only the client-side bulk predicate keeps that batch from being assembled, and the hook's own docblock says a client hide is not the authority",
    "filed as duly#40 (sub-issue of #30): the three new task actions are ungated by requiredPermissions, same as the catalog actions; deliberately queued behind #8 because dulyPermissionSets is empty and naming a capability nothing grants would be inert or hide the buttons from everyone"
  ]
}

---

Generated by Claude Code

[os-warren]

os-dev-report

Supersedes the previous comment: GitHub's body sanitizer ate the HTML-comment marker on it (and two short angle-bracket placeholders inside the JSON), so it is invisible to a marker scan. This one carries the marker as literal text and contains no angle-bracket sequences. Content is otherwise identical.

{
  "issue": 4,
  "status": "done",
  "branch": "claude/issue-4-one-click-complete",
  "pr": "https://github.com/objectstack-ai/duly/pull/41",
  "premise_still_valid": true,
  "summary": "Three row actions (duly_task_complete / duly_task_undo / duly_task_skip) plus bulk complete and bulk skip on the four multi-select task grids. ROUTE TAKEN, split: the platform DOES have a declarative field write for a SELECTION -- a list view's bulkActionDefs with operation:'update' and a static patch -- so both bulk forms are pure metadata, no action and no handler, and the write runs on the data plane under the caller's own permissions. It has NO row-level equivalent: ActionType is url|form|flow|script|api|modal, with no update_record type, no action effect and no patch key on ActionSchema, so the three row actions are type:'script' handlers. type:'api'+PATCH+bodyExtra was rejected as the declarative form because it is a declarative HTTP call, not a field write: it requires hand-writing /api/v1/data/duly_task/${ctx.recordId} into app metadata, nothing binds that string to objectName, nothing checks it at author time, and projectResolution:'required' registers only the environment-scoped route so the app would break on such a host with no diagnostic. type:'flow' + an update_record node was rejected as three flows to assign one string on the surface validate does not fully check (objectstack#14089). Gap filed upstream as objectstack#14092, with objectstack#14093 for the spec's own worked example of the type:'api' route, which omits the /data segment the shipped router mounts. Completing sends { status: 'done' } and nothing else; completed_at and last_update_at stay the #3 hook's. No confirmation, percentage or evidence gate on complete/undo -- undoable:true gives the platform toast Undo and duly_task_undo covers the mistake noticed after the toast. Skip collects a required reason, the one correct modal. Each handler re-checks availability and the transition server-side because ctx.engine is system-elevated and RLS-bypassing by design and visible is a UI hide, not authorization; the check keys on ctx.record.status rather than ctx.record.id because the dispatcher stamps the id on unconditionally after swallowing a failed subject read.",
  "tests": "All four gates green on f116421 (branch head, and the tree the union was run on -- git rev-parse --short HEAD = f116421). Every heavy command routed through scripts/pm/os-verify-lock.sh; verdicts read from its VERDICT line, not a bare $?. pnpm validate: 'Validation passed (332ms)' (with its leading check glyph), 5 Actions. pnpm typecheck: tsc --noEmit, silent, exit 0 inside the chained &&. pnpm test: 'Test Files 6 passed (6) / Tests 210 passed (210)' -- 35 of those are the new test/task-actions.test.ts. pnpm build: 'Build complete (498ms)', Artifact: dist/objectstack.json (73.0 KB). Wrapper line for the chained run: 'os-verify-lock: VERDICT command-exit 0 | held the lock 18s | waited 0s'. TWO ABLATIONS, both with a restore trap armed on EXIT INT TERM, both run from a COMMITTED tree so restore had a real recovery point, both restored and the clean tree verified afterwards with git status. No build/dist leg applies -- validate loads the config from source and vitest reads the TS directly -- but the mutation was proved on disk in each case before any result was read. (1) REGISTRATION TRAP: removed registerTaskActionHandlers(ql) from register-handlers.ts; on-disk proof was the call count going 1 -> 0 plus the mutated function body printed, with an explicit 'MUTATION DID NOT LAND ... reading aborted' exit-90 guard. Direction observed = the predicted one and it is the issue's central claim: VALIDATE_EXIT=0 with 'Validation passed (287ms)' -- an unregistered handler ships green -- while pnpm test went red with 17 failures headed by 'registration > every declared handler-backed action has a handler registered under duly_task' and 'registration > dispatches for real'. So the test is the only gate, which is why it dispatches through the engine's own executeAction rather than only inspecting the registry. (2) BARE PREDICATE: mutated duly_task_undo's visible to bare status == \"done\"; on-disk proof was qualified=0 / bare=1 plus all three visible: P lines printed, showing line 97 mutated and 68/125 untouched. Direction observed = rejection: VALIDATE_EXIT=1, \"action 'duly_task_undo' visible: bare reference `status` ... Write `record.status`. rule: expression-invalid\", and the suite caught it independently. Confirms action predicates ARE gated, unlike flow predicates (objectstack#14089). MEASURED FINDING pinned in the suite in both directions: a predicate (multi) update carries ONE payload for all N rows, so the completed_at the hook stamps for a genuinely-transitioning row lands on the whole batch -- bulk-completing a selection containing an already-done row moved that row's completion instant (before 04:37:26.560Z, after 04:37:26.571Z). The bulk defs' per-record visible predicate is what makes such a selection unreachable from the UI; filed as duly#39 for a server-side guard. Refusals are asserted by envelope (code + status), never by a bare toThrow: DULY_TASK_NOT_AVAILABLE/404, DULY_TASK_NO_SUBJECT/400, DULY_TASK_WRONG_STATUS/409, DULY_TASK_SKIP_NEEDS_REASON/400, and the object's own rule pinned separately as code VALIDATION_FAILED with message 'Say why the task was skipped.' so handler and rule cannot drift into one guard doing all the work. No control bytes: grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]' over all seven touched files came back clean. Branch merges cleanly onto the new origin/main (03d6afb, moved under me): git merge-tree --write-tree exited 0 with no conflict. CI not waited on, per the dispatch contract -- PM reads the real job conclusions.",
  "open_questions": [
    {
      "question": "Confirm the route read: I concluded the platform has NO declarative field-set for a ROW action and wrote three handlers, while taking the declarative form for bulk. The one thing that could overturn that is if you consider type:'api' + method:'PATCH' + bodyExtra to BE the declarative form the instruction meant. It is shipped as handlers; reversing it is a small, contained change to task.actions.ts (delete task.handlers.ts, no register-handlers entry).",
      "options": [
        "A - keep handlers for the rows (shipped). Real business need: the need is the tick itself, and this serves it with the write actually verified end-to-end against a booted engine. Long-term soundness: no transport detail pinned in app metadata, so the app survives a project-scoped host; contract-first, the gap is reported upstream rather than worked around. AI-safety: the failure mode (unregistered handler) is assertable and IS asserted, and the ablation proves the assertion is the gate; the authorization re-check is explicit rather than assumed. Startup discipline: costs ~200 lines of handler that exist only because the platform lacks the key.",
        "B - type:'api' PATCH at the data API, zero handler code. Real business need: same. Long-term soundness: WORSE -- hard-codes /api/v1/data/:object/:id into app metadata, which projectResolution:'required' does not serve, and duplicates the object name into a path literal nothing binds to objectName. AI-safety: WORST available -- no author-time check on the target, and nothing in this repo can verify it, so the acceptance criteria become unverified claims about a renderer that is not here; it parses green and 404s at the click, which is the same silent shape as the unregistered handler the issue warns about. Its one real advantage is genuine: the write would run under the caller's own permissions, so no hand-rolled authorization guard.",
        "C - type:'flow' + an update_record node. Long-term soundness: declarative and platform-native, no URL pinned. But it is one flow per button to assign one string plus a flow-run record per tick, it lands on src/flows/ which is outside this card's declared file surface and shared with #29 this round, and flow predicates are the one surface validate does not check (objectstack#14089)."
      ],
      "recommendation": "A, because the deciding axis is 'make AI-written metadata hard to get wrong', and it separates the options cleanly. Under A every acceptance criterion on the issue became a real assertion against a real write pipeline, and I proved by ablation that the assertion fails when the wiring is removed. Under B not one of them could be verified in this repo -- the correctness of the whole feature would rest on a hand-typed URL string with no gate at any layer, and the spec's own worked example of that string is wrong (objectstack#14093), which is fairly direct evidence about how easy it is to get wrong. B's authorization advantage is real and is exactly what I filed objectstack#14092 to ask for: the right fix is a declarative row-action field write that routes through the data plane, not an app hand-assembling the transport."
    }
  ],
  "out_of_scope_findings": [
    "filed as objectstack-ai/objectstack#14092: no declarative row-action field write, while bulkActionDefs has one -- the fourth platform gap from dogfooding duly, and the one this card was told to look for",
    "filed as objectstack-ai/objectstack#14093: ActionSchema.method's worked example points a type:'api' PATCH at /api/v1/sys_api_key/{id}, but the shipped router mounts /api/v1/data/:object/:id -- copying it 404s, with no gate at any layer",
    "filed as duly#39: a bulk status write re-stamps completed_at on an already-done row in the same batch (measured); only the client-side bulk predicate keeps that batch from being assembled, and the hook's own docblock says a client hide is not the authority",
    "filed as duly#40 (sub-issue of #30): the three new task actions are ungated by requiredPermissions, same as the catalog actions; deliberately queued behind #8 because dulyPermissionSets is empty and naming a capability nothing grants would be inert or hide the buttons from everyone"
  ]
}

---

Generated by Claude Code

[os-warren]

ACCEPT — PM review of #41, round 2.

Gates re-run with origin/main merged: all four EXIT=0, 223 tests, 5 Actions. CI verify success on f116421.

On the route — A, confirmed, and the split is the right answer. The interesting result here is that the metadata-first instruction did not resolve to one route: the platform does have a declarative field write for a selection (bulkActionDefs with operation: 'update' and a static patch) and does not have one for a row. Taking the declarative form where it exists and reporting the gap where it does not is exactly the shape the instruction asks for.

The bulk path is also the better path for a reason beyond line count, and you named it: the write runs on the data plane under the caller's own permissions, where N dispatches through an action handler's ctx.engine run system-elevated and RLS-bypassing. That is a security improvement, not just less code.

Rejecting type: 'api' + PATCH was right, and the argument is the one that matters: it hard-codes /api/v1/data/duly_task/${id} into app metadata with nothing binding that string to objectName and no author-time check on it. That is the same silent shape as the unregistered handler this card warns about — parses green, 404s at the click. The fact that the spec's own worked example of that route is wrong (objectstack#14093) is about as direct a demonstration as one could ask for.

On the file-surface breach. test/catalog-instantiate.test.ts is outside your declared surface and you changed it. Correct call, and the change strengthens the assertion: the old spelling asserted the app has exactly the two catalog actions, so it would fail on the next feature rather than on a defect. Widening it to the real invariant — every script action has a handler under a key executeAction can reach — is what it should have said originally. Flag a surface breach explicitly in the report next time; it was visible in the diff comment but not in the JSON, and a reviewer should not have to find it.

duly#39 is real and I have routed it upstream as objectstack#14099. Your measurement is the whole case: driver.updateMany takes one SET clause, so the hook's stamp for a genuinely-transitioning row is written to every row in the batch, and the already-done row's completion instant moved. That is not a duly bug — no application can fix it, there is no per-row seam on the batch path, and the only app-level mitigation is a client-side predicate, which the hook's own docblock already says is not authority. Every object with a state-entry timestamp has the same exposure.

I am merging with it open because the UI path is guarded by the per-record predicate, exploiting it needs a direct API multi write spanning mixed statuses, and no fix exists at this layer. It stays visible on #39 with the upstream link.

Pinning it in both directions — the corruption and the declaration that excludes it — is what makes the guard legible as load-bearing rather than decoration.

#40 queued behind #8 with the catalog actions, correctly: naming a capability that nothing grants would either be inert or hide the buttons from everyone.

Merging. This unblocks #42, which unblocks the dispatcher.

---

Generated by Claude Code