Resident-owned durable wake-ups for task follow-through #22
No reviewers
Labels
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
key-store/dispatch!22
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "codex/assistant-wakeups"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The assistant could receive founder SMS but could not wake itself to follow through on commitments (Redmine #43). Add identity-owned one-time and recurring schedules in the existing resident queue, with authenticated hub control and normal resident execution/spending limits.
Records use owner isolation, stable IDs and optimistic revisions. Due claims and steer insertion are atomic across PostgreSQL API hosts; missed ticks and an existing pending occurrence coalesce. One-time/retired-owner records pause. Queued work survives edits/removal. Scheduled payloads retain resident authorship and existing authority.
Control requires loopback plus an endpoint/timestamp/body-bound HMAC using the existing host secret; org membership alone does not authenticate. Management is exposed through the paired key.store MCP tools. No outbound SMS path is added.
Validation: Nix full Go suite passed on the initial implementation with SQLite and an isolated PostgreSQL cluster. Final stable-ID recreation regression is being rerun on this exact head; do not merge until final checks and security review are recorded. Paired key.store changes add the PostgreSQL check setup, hub tools, KSE decision and role prompts. KSE-0199 peer subscriptions remain separate.
Rollback: roll back dispatcher/hub or pause/remove schedules; additive table is ignored by earlier code. Preserve queued work and notebooks.
Senior security review — exact head
4be3e57— APPROVE (review only, no merge/deploy).Scope reviewed:
internal/agentdispatcher/schedules.go(new, 374),schedules_test.go(new, 285),api.go/queue.go/steer.gowiring. Verified against the live head4be3e5781e338898f5a3af63a47568e593498387; PR is mergeable, no prior reviews.Handler auth (loopback AND HMAC, no unsigned bypass) — pass.
scheduleControlHandlerrequiresisLoopbackRequest(RemoteAddr-based vianet.SplitHostPort+IsLoopback; noX-Forwarded-Fortrust) ANDHMAC-SHA256(secret, stamp + "\n" + "/control/schedules" + "\n" + body)with the host secret; empty secret forces 403. Skew window 5 min past / 30 s future, 16 KiB body cap,DisallowUnknownFieldsplus trailing-data rejection. Existing webhook verifiers use different MAC constructions and headers, and the endpoint string is bound into the MAC, so a captured webhook signature cannot be replayed at the control endpoint.TestResidentScheduleControlAuthenticationpins all of this: unsigned, off-loopback, wrong-path signature, stale timestamp,founderowner, and unassigned identity are all 403.Owner scoping / nonfounder — pass. Owner is slug-normalized and must satisfy
isResidentActor(org assignment required,founderexplicitly excluded) plus a non-emptyprimaryRoleForIdentity. Every mutation bindsWHERE owner, list is owner-scoped, and the lifecycle test proves cross-owner list/removal are rejected.Schedule identity + revision isolation — pass. Update path preserves the stable schedule ID, bumps
revision, and applies a conditionalUPDATE ... WHERE id AND owner AND revisionwithRowsAffected == 1enforcement; create requiresExpectedRevision == 0/empty ID, update/remove require exactExpectedID/ExpectedRevision. Lost-update and cross-owner conflicts surface as 409/404, never silent overwrite.Atomic advance+enqueue across hosts — pass.
fireScheduleruns the conditional clock-advance, retirement check, pending check, and steer insert in one transaction; only one host can win theUPDATE(revision +next_run_nsmatch), andTestResidentScheduleAtomicRollbackproves a failed enqueue does not advance the clock (retry still fires exactly once). Quota checks take apg_advisory_xact_lockon PostgreSQL; SQLite writers serialize. Race coverage runs on both SQLite and isolated-schema PostgreSQL.Pending/missed coalescing — pass. Missed ticks collapse to the next future slot (
(elapsed/interval + 1) * interval); an owner with a pendingschedule:<id>:%steer gets no duplicate;event_keyisUNIQUEsoON CONFLICT DO NOTHINGis sound on both drivers. Late-poll collapse is pinned by test.Retirement + preserved authorship — pass. Retired owners (no assignment/role) get their record paused with no enqueue; one-shots pause after firing (
next_run_nsunchanged,paused=1, no refire path). Enqueued payload carriesowner_identity, schedule id/name, task, andtrust: resident-self-scheduled, and thesteer.goprompt change preserves original authority. The file header correctly frames a wake as an opportunity to work, not an authorization.Input bounds — pass. Name pattern, task 1..8000 bytes,
every_seconds0 or 300..2592000,run_atwithin the next year and ≥60 s future when active, 16 schedules per owner, revision guards. Bounds + quota + retirement pinned byTestResidentScheduleBoundsAndRetirement.Notes (non-blocking): (1)
FireDueSchedulesaborts the remaining due batch on the first fire error — liveness nit, errors surface in the runner log; consider continue-on-error later. (2) No Go toolchain in the review sandbox, so tests were inspected, not executed here — this approval relies on the reported Nix full-suite pass; recommend merging only after the running stable-ID regression is green. (3) Commit carries an SSH signature blob that this sandbox cannot verify (no allowedSignersFile); Forgejo's verification badge should be confirmed at merge. (4) Paired MCP/auth/prompt/Nix changes are out of scope here and need their own review.Security approval granted for dispatch PR #22 at exact head
4be3e57. Merge/deploy remains with the authorized merger; this review performed no merge, no deploy, and no secret/production actions.Senior security review — exact head
4be3e57— APPROVE (review only, no merge/deploy). Full findings: handler auth loopback+HMAC (stamp/endpoint/body binding, skew 5m/30s, 16KiB cap, strict JSON) with no unsigned bypass; owner slug-normalized, isResidentActor with explicit founder exclusion + primary role required, all mutations owner-bound; stable schedule ID + revision optimistic concurrency (409/404, no silent overwrite); atomic advance+enqueue in one tx with conditional UPDATE (single-host win) + advisory lock for quota on pg, rollback test proves no clock-advance without enqueue; missed-tick collapse + pending coalescing via UNIQUE event_key; retirement pauses with no enqueue, authorship preserved (owner_identity/task/trust fields, steer prompt preserves original authority, wake != authorization); input bounds + 16/owner quota. Tests pin unsigned/off-loopback/wrong-path/stale/founder/unassigned rejections, cross-owner isolation, races on sqlite+pg, rollback, bounds/retirement. Non-blocking notes: batch abort on first fire error; tests inspected not executed here (no Go toolchain) — merge after stable-ID regression green; SSH sig blob unverifiable in sandbox — confirm Forgejo badge at merge; paired MCP/auth/prompt/Nix changes need separate review. Approval for dispatch PR22 at exact head4be3e57only.Full findings for senior security approval (review 4676, APPROVED at exact head
4be3e57). Scope:internal/agentdispatcher/schedules.go(new, 374),schedules_test.go(new, 285),api.go/queue.go/steer.gowiring. Head verified4be3e5781e338898f5a3af63a47568e593498387; PR mergeable; no prior reviews.Handler auth (loopback AND HMAC, no unsigned bypass) — pass.
scheduleControlHandlerrequiresisLoopbackRequest(RemoteAddr-based vianet.SplitHostPort+IsLoopback; noX-Forwarded-Fortrust) ANDHMAC-SHA256(secret, stamp + "\n" + "/control/schedules" + "\n" + body)with the host secret; empty secret forces 403. Skew window 5 min past / 30 s future, 16 KiB body cap,DisallowUnknownFieldsplus trailing-data rejection. Existing webhook verifiers use different MAC constructions and headers, and the endpoint string is bound into the MAC, so a captured webhook signature cannot be replayed at the control endpoint.TestResidentScheduleControlAuthenticationpins all of this: unsigned, off-loopback, wrong-path signature, stale timestamp,founderowner, and unassigned identity are all 403.Owner scoping / nonfounder — pass. Owner is slug-normalized and must satisfy
isResidentActor(org assignment required,founderexplicitly excluded) plus a non-emptyprimaryRoleForIdentity. Every mutation bindsWHERE owner, list is owner-scoped, and the lifecycle test proves cross-owner list/removal are rejected.Schedule identity + revision isolation — pass. Update path preserves the stable schedule ID, bumps
revision, and applies a conditionalUPDATE ... WHERE id AND owner AND revisionwithRowsAffected == 1enforcement; create requiresExpectedRevision == 0/empty ID, update/remove require exactExpectedID/ExpectedRevision. Lost-update and cross-owner conflicts surface as 409/404, never silent overwrite.Atomic advance+enqueue across hosts — pass.
fireScheduleruns the conditional clock-advance, retirement check, pending check, and steer insert in one transaction; only one host can win theUPDATE(revision +next_run_nsmatch), andTestResidentScheduleAtomicRollbackproves a failed enqueue does not advance the clock (retry still fires exactly once). Quota checks take apg_advisory_xact_lockon PostgreSQL; SQLite writers serialize. Race coverage runs on both SQLite and isolated-schema PostgreSQL.Pending/missed coalescing — pass. Missed ticks collapse to the next future slot (
(elapsed/interval + 1) * interval); an owner with a pendingschedule:<id>:%steer gets no duplicate;event_keyisUNIQUEsoON CONFLICT DO NOTHINGis sound on both drivers. Late-poll collapse is pinned by test.Retirement + preserved authorship — pass. Retired owners (no assignment/role) get their record paused with no enqueue; one-shots pause after firing (
next_run_nsunchanged,paused=1, no refire path). Enqueued payload carriesowner_identity, schedule id/name, task, andtrust: resident-self-scheduled, and thesteer.goprompt change preserves original authority. The file header correctly frames a wake as an opportunity to work, not an authorization.Input bounds — pass. Name pattern, task 1..8000 bytes,
every_seconds0 or 300..2592000,run_atwithin the next year and >=60 s future when active, 16 schedules per owner, revision guards. Bounds + quota + retirement pinned byTestResidentScheduleBoundsAndRetirement.Notes (non-blocking): (1)
FireDueSchedulesaborts the remaining due batch on the first fire error — liveness nit, errors surface in the runner log; consider continue-on-error later. (2) No Go toolchain in the review sandbox, so tests were inspected, not executed here — this approval relies on the reported Nix full-suite pass; recommend merging only after the running stable-ID regression is green. (3) Commit carries an SSH signature blob that this sandbox cannot verify (no allowedSignersFile); Forgejo's verification badge should be confirmed at merge. (4) Paired MCP/auth/prompt/Nix changes are out of scope here and need their own review.Security approval granted for dispatch PR #22 at exact head
4be3e57. Merge/deploy remains with the authorized merger; this review performed no merge, no deploy, and no secret/production actions.Final validation on dispatch
4be3e5781e338898f5a3af63a47568e593498387and key.storef47f15f6452fe7f5d6816fd005ce144d1bc4506cpassed:/nix/store/dd3fk9xs1k2n1yn90gqz9s1i9774xxij-agent-dispatcher-0.1.0.drv; checkPhase started PostgreSQL and passed internal/agentdispatcher and internal/harness.Runtime integration is ready to merge. Production schedules remain disabled by absence of records until the paired reviewed rollout; live same-session self-wake and founder follow-through verification remain pending.