feat(api): proposal approval verification and ordered application #70

Merged

yuris merged 1 commit from feat/issue-27-proposal into main 2026-09-23 03:01:03 +00:00

Conversation 1 Commits 1 Files changed 18 +3979 -25

yuris commented 2026-09-23 02:59:00 +00:00

Owner

Copy link

Motivation

Issue #25 stores a reviewable proposal and #26 replans around a reported interruption, but nothing could act on a proposal. This lands the missing half of the approval contract in docs/technical/cli-api.md#idempotency-and-approval: a proposal is applied only against the review that was actually performed, every baseline is revalidated live before anything is dispatched, the steps execute in their immutable order each as its own child operation, and a step that does not land leaves an explicitly partial proposal instead of a blind rollback or an automatic retry.

Scenarios R09 (a step conflicts after preflight), R14 (an unreviewed, stale or expired proposal) and R15 (a re-application after partial success) are the acceptance boundary.

Changes

• src/api/proposals.rs (new) — the three routes and the whole workflow:
  • GET /v1/proposals/{id}: the stored proposal with its state, digest, lifetime, ordered steps (target, intended change, baseline version, reason) and the recorded policy exceptions. A pending proposal whose review window has closed reads expired; nothing is written by the read.
  • POST /v1/proposals/{id}/apply: the approval ({approval:{confirmed:true,proposal_digest,reviewed_at}}) is verified before anything is read; the digest, expiry and stored state decide applicability; then every step's baseline ETag and every creation precondition are revalidated live, and the first mismatch refuses the whole apply naming that step with nothing dispatched. Only then does the parent operation commit its intent, and the steps run in order.
  • POST /v1/proposals/{id}/reject: one local transaction marking the proposal rejected with the optional reason. No CalDAV client is constructed on this path.
• src/api/update.rs / src/api/creation.rs — a step is dispatched through exactly the paths a direct mutation uses (update_task_as_step, create_event_as_step), with the step's own operation identity and the reviewed baseline as the conditional precondition. The step link travels with the call, so no parallel writer exists.
• src/history/journal.rs — StepLink plus a step argument on prepare_operation, written with the prepared intent (so a crash cannot leave a dispatched step unlinked); step_operations reads one parent's children.
• src/history/proposals.rs — commit_apply_outcome (the parent operation's state and the proposal's state in one transaction, so the durable record can never disagree about whether the proposal was applied) and record_rejection.
• src/history/recovery.rs — a multi-step parent is reconciled from its recorded children rather than from a live readback of a single target it never had.
• src/api/mod.rs — approval_required (403/3), proposal_expired (400/2) and proposal_not_applicable (409/4) failure shapes, three handlers and three routes (auth::route_access already mapped them, verified).
• src/cli.rs / src/client.rs — life proposal get|apply|reject, with the daemon's own approval rule applied before transmission and the documented exit codes (3 unreviewed/mismatched, 2 expired, 4 not applicable, 6 partial).
• src/domain/proposal.rs — parse_steps and step_operation_id (the child identity rule: parent:step).

Behaviour decisions worth naming:

• A step that fails or whose outcome is unknown stops the apply: the applied steps stay applied, the failed step is failed, the remaining steps are pending, the parent is needs_reconciliation, the proposal is partial and the answer is partial_apply/exit 6. Nothing is rolled back over whatever the calendar holds now, and nothing is retried. The proposal is marked partial even when no step was confirmed applied — the safe reading of "repair needs a newly reviewed proposal" once a dispatch has been attempted.
• Same key and same request replays the recorded outcome without dispatching a step; same key and a changed request is idempotency_conflict. A replay of an apply whose outcome was never recorded (crash between a step's dispatch and the parent's commit) reports the progress its child operations prove and dispatches nothing.
• An uncertain step (unknown/needs_reconciliation) is reported as unknown with its operation ID, never as applied.
• Expiry is enforced on both transitions: an expired proposal is neither applied nor rejected, and no state transition is recorded after its review window closed.
• reviewed_at and the verified digest are recorded as audit evidence. Review time is not proof a human was present: the module doc, the approval_required message and the CLI footer all state that the authorized client remains responsible for the review it supplied.

Deviations from the issue text, all deliberate:

• proposal_not_applicable is HTTP 409 / exit 4 for every non-applicable state (applied, partial, rejected). The issue's prose says 400 for applied/rejected, but its own CLI mapping requires exit 4, and docs/technical/cli-api.md maps exit 4 to 409/412; one code cannot carry two mappings.
• proposal_expired is HTTP 400 / exit 2, per the issue's acceptance criterion and the exit table (exit 3 is the approval/clarification class, which an expired proposal cannot resolve).
• The proposal's operation_id column keeps the operation that generated it; the operation that decided it (apply or rejection) is recorded in approval_json beside the decision, so generation provenance is never overwritten.
• A proposal that allocated nothing applies as a confirmed no-op (state: applied, steps: [], zero writes) rather than being refused, per the operation state machine's no-op rule.
• prepare_operation gained a step: Option<&StepLink> argument; the existing callers pass None (the link is written with the intent rather than patched in afterwards).

Verification

cargo test — 654 tests pass, 0 failures (the suite was 633 before; the 21 new tests are below). cargo clippy --all-targets -- -D warnings and cargo fmt --check are clean. No dependency was added or changed.

tests/proposal_apply_tests.rs (new, 21 tests) observes everything through the real path — the proposal is generated by the daemon's planner against a mock CalDAV server over TCP, applied through the daemon's own router, and read back from SQLite:

• R14: an apply with no approval, an unconfirmed approval, a missing digest, an unusable reviewed_at, a stale digest and an expired proposal are each refused with their documented code and status, zero requests of any kind reach the backend, and the journal row counts are unchanged.
• R14 (preflight): a task retagged by "another client" and a creation target that already exists each refuse the whole apply with etag_conflict naming the step, with only reads issued and zero PUTs.
• R09: with the second write answered 412, the four-step proposal reports ["applied", "failed", "pending", "pending"], HTTP 200 / status partial / partial_apply with the operation ID, the first step's write still on the backend and its ETag reported, exactly two PUTs and zero DELETEs, the proposal partial, the parent needs_reconciliation, and both child operations linked by parent_operation_id + step_id. The dispatched If-Match is asserted to be the reviewed baseline, and the creation's If-None-Match: *.
• R15 / R08: re-applying the partial proposal under a fresh key is proposal_not_applicable with no request issued; the same key and payload replays the recorded partial outcome with no second PUT; the same key with a changed request is idempotency_conflict.
• Uncertain step: a 500 from the backend leaves the step unknown with operation_unknown and its operation ID (its own operation recorded needs_reconciliation), the proposal partial, and the remaining steps pending.
• Interrupted apply: a seeded prepared parent with one applied child replays as partial/operation_unknown reporting that step, dispatching nothing.
• Startup recovery reconciles a multi-step parent from its children with no live readback.
• Rejection marks the proposal rejected with its reason, touches no calendar resource, replays under the same key, and refuses a second rejection; an expired proposal cannot be rejected.
• CLI: proposal get renders the digest and steps, proposal apply exits 3 for an unreviewed payload without contacting the daemon, exits 6 with the per-step breakdown for a partial apply, exits 4 for a re-application, and proposal reject records the decision.

Related Issues

Closes #27

## Motivation Issue #25 stores a reviewable proposal and #26 replans around a reported interruption, but nothing could act on a proposal. This lands the missing half of the approval contract in `docs/technical/cli-api.md#idempotency-and-approval`: a proposal is applied only against the review that was actually performed, every baseline is revalidated live before anything is dispatched, the steps execute in their immutable order each as its own child operation, and a step that does not land leaves an explicitly partial proposal instead of a blind rollback or an automatic retry. Scenarios R09 (a step conflicts after preflight), R14 (an unreviewed, stale or expired proposal) and R15 (a re-application after partial success) are the acceptance boundary. ## Changes - `src/api/proposals.rs` (new) — the three routes and the whole workflow: - `GET /v1/proposals/{id}`: the stored proposal with its state, digest, lifetime, ordered steps (target, intended change, baseline version, reason) and the recorded policy exceptions. A `pending` proposal whose review window has closed reads `expired`; nothing is written by the read. - `POST /v1/proposals/{id}/apply`: the approval (`{approval:{confirmed:true,proposal_digest,reviewed_at}}`) is verified before anything is read; the digest, expiry and stored state decide applicability; then **every** step's baseline ETag and every creation precondition are revalidated live, and the first mismatch refuses the whole apply naming that step with nothing dispatched. Only then does the parent operation commit its intent, and the steps run in order. - `POST /v1/proposals/{id}/reject`: one local transaction marking the proposal `rejected` with the optional reason. No CalDAV client is constructed on this path. - `src/api/update.rs` / `src/api/creation.rs` — a step is dispatched through exactly the paths a direct mutation uses (`update_task_as_step`, `create_event_as_step`), with the step's own operation identity and the reviewed baseline as the conditional precondition. The step link travels with the call, so no parallel writer exists. - `src/history/journal.rs` — `StepLink` plus a `step` argument on `prepare_operation`, written with the prepared intent (so a crash cannot leave a dispatched step unlinked); `step_operations` reads one parent's children. - `src/history/proposals.rs` — `commit_apply_outcome` (the parent operation's state and the proposal's state in **one** transaction, so the durable record can never disagree about whether the proposal was applied) and `record_rejection`. - `src/history/recovery.rs` — a multi-step parent is reconciled from its recorded children rather than from a live readback of a single target it never had. - `src/api/mod.rs` — `approval_required` (403/3), `proposal_expired` (400/2) and `proposal_not_applicable` (409/4) failure shapes, three handlers and three routes (`auth::route_access` already mapped them, verified). - `src/cli.rs` / `src/client.rs` — `life proposal get|apply|reject`, with the daemon's own approval rule applied before transmission and the documented exit codes (3 unreviewed/mismatched, 2 expired, 4 not applicable, 6 partial). - `src/domain/proposal.rs` — `parse_steps` and `step_operation_id` (the child identity rule: `parent:step`). Behaviour decisions worth naming: - A step that fails or whose outcome is unknown stops the apply: the applied steps stay applied, the failed step is `failed`, the remaining steps are `pending`, the parent is `needs_reconciliation`, the proposal is `partial` and the answer is `partial_apply`/exit 6. Nothing is rolled back over whatever the calendar holds now, and nothing is retried. The proposal is marked `partial` even when no step was confirmed applied — the safe reading of "repair needs a newly reviewed proposal" once a dispatch has been attempted. - Same key and same request replays the recorded outcome without dispatching a step; same key and a changed request is `idempotency_conflict`. A replay of an apply whose outcome was never recorded (crash between a step's dispatch and the parent's commit) reports the progress its child operations prove and dispatches nothing. - An uncertain step (`unknown`/`needs_reconciliation`) is reported as `unknown` with its operation ID, never as applied. - Expiry is enforced on both transitions: an expired proposal is neither applied nor rejected, and no state transition is recorded after its review window closed. - `reviewed_at` and the verified digest are recorded as audit evidence. Review time is not proof a human was present: the module doc, the `approval_required` message and the CLI footer all state that the authorized client remains responsible for the review it supplied. Deviations from the issue text, all deliberate: - `proposal_not_applicable` is HTTP 409 / exit 4 for every non-applicable state (applied, partial, rejected). The issue's prose says 400 for applied/rejected, but its own CLI mapping requires exit 4, and `docs/technical/cli-api.md` maps exit 4 to 409/412; one code cannot carry two mappings. - `proposal_expired` is HTTP 400 / exit 2, per the issue's acceptance criterion and the exit table (exit 3 is the approval/clarification class, which an expired proposal cannot resolve). - The proposal's `operation_id` column keeps the operation that *generated* it; the operation that *decided* it (apply or rejection) is recorded in `approval_json` beside the decision, so generation provenance is never overwritten. - A proposal that allocated nothing applies as a confirmed no-op (`state: applied`, `steps: []`, zero writes) rather than being refused, per the operation state machine's no-op rule. - `prepare_operation` gained a `step: Option<&StepLink>` argument; the existing callers pass `None` (the link is written with the intent rather than patched in afterwards). ## Verification `cargo test` — 654 tests pass, 0 failures (the suite was 633 before; the 21 new tests are below). `cargo clippy --all-targets -- -D warnings` and `cargo fmt --check` are clean. No dependency was added or changed. `tests/proposal_apply_tests.rs` (new, 21 tests) observes everything through the real path — the proposal is generated by the daemon's planner against a mock CalDAV server over TCP, applied through the daemon's own router, and read back from SQLite: - **R14**: an apply with no approval, an unconfirmed approval, a missing digest, an unusable `reviewed_at`, a stale digest and an expired proposal are each refused with their documented code and status, **zero requests of any kind reach the backend**, and the journal row counts are unchanged. - **R14 (preflight)**: a task retagged by "another client" and a creation target that already exists each refuse the whole apply with `etag_conflict` naming the step, with only reads issued and zero `PUT`s. - **R09**: with the second write answered `412`, the four-step proposal reports `["applied", "failed", "pending", "pending"]`, HTTP 200 / status `partial` / `partial_apply` with the operation ID, the first step's write still on the backend and its ETag reported, exactly two `PUT`s and zero `DELETE`s, the proposal `partial`, the parent `needs_reconciliation`, and both child operations linked by `parent_operation_id` + `step_id`. The dispatched `If-Match` is asserted to be the reviewed baseline, and the creation's `If-None-Match: *`. - **R15 / R08**: re-applying the partial proposal under a fresh key is `proposal_not_applicable` with no request issued; the same key and payload replays the recorded partial outcome with no second `PUT`; the same key with a changed request is `idempotency_conflict`. - Uncertain step: a `500` from the backend leaves the step `unknown` with `operation_unknown` and its operation ID (its own operation recorded `needs_reconciliation`), the proposal `partial`, and the remaining steps pending. - Interrupted apply: a seeded prepared parent with one applied child replays as `partial`/`operation_unknown` reporting that step, dispatching nothing. - Startup recovery reconciles a multi-step parent from its children with no live readback. - Rejection marks the proposal `rejected` with its reason, touches no calendar resource, replays under the same key, and refuses a second rejection; an expired proposal cannot be rejected. - CLI: `proposal get` renders the digest and steps, `proposal apply` exits 3 for an unreviewed payload without contacting the daemon, exits 6 with the per-step breakdown for a partial apply, exits 4 for a re-application, and `proposal reject` records the decision. ## Related Issues Closes #27

yuris added 1 commit 2026-09-23 02:59:00 +00:00

feat(api): proposal approval verification and ordered application e83638dcab

yuris commented 2026-09-23 03:01:02 +00:00

Author

Owner

Copy link

Coordinator review — APPROVED

Verified e83638d myself: cargo test 654 passed / 0 failed (21 new proposal-apply tests), cargo clippy --all-targets -- -D warnings 0 errors, cargo fmt --check clean.

What I checked beyond the summary:

• The refusal ordering is the right way round. run_proposal_apply validates the operation key, body and approval evidence, then requires the durable journal before anything else ("without it no step intent could be committed, so no step may be dispatched"), then loads the proposal, computes reviewability (expiry/state/digest), preflights, and only then commits the parent intent and dispatches steps. Every refusal path tests request_count() == requests_before and unchanged journal counts, so "zero writes" means no request of any kind, not just no PUT.
• Preflight happens before the parent intent exists, and it covers creation steps (nonexistence by derived target URL) as well as updates — so a drift discovered late cannot produce a half-applied proposal. The dispatched If-Match is asserted to equal the reviewed baseline, which is what makes the digest meaningful rather than decorative.
• R09 is demonstrated exactly as specified: statuses [applied, failed, pending, pending] with the proposal partial, the parent needs_reconciliation, children linked by (parent, step_id), exactly two PUTs and zero DELETEs, and the applied step's write still on the backend. No rollback, no retry — the failure is recorded, not undone.
• A stale proposal cannot be acted on: a pending row past its window is reported as expired (a computed view rather than a stored lie), so the state cannot be stale in the one place that matters.
• Reject never touches the calendar — no CalDAV client is constructed on that path at all, which is stronger than asserting no request was made — and re-reject/apply yields proposal_not_applicable.
• Approval is treated as audit evidence (reviewed_at + verified digest recorded), consistent with the contract's position that the authorized client, not the service, carries responsibility for review.

Merging. #28 (outcomes) and #31 (agent skill) are now unblocked.

## Coordinator review — APPROVED Verified `e83638d` myself: `cargo test` **654 passed / 0 failed** (21 new proposal-apply tests), `cargo clippy --all-targets -- -D warnings` 0 errors, `cargo fmt --check` clean. What I checked beyond the summary: - **The refusal ordering is the right way round.** `run_proposal_apply` validates the operation key, body and approval evidence, then requires the durable journal *before* anything else ("without it no step intent could be committed, so no step may be dispatched"), then loads the proposal, computes reviewability (expiry/state/digest), preflights, and only then commits the parent intent and dispatches steps. Every refusal path tests `request_count() == requests_before` and unchanged journal counts, so "zero writes" means *no request of any kind*, not just no `PUT`. - **Preflight happens before the parent intent exists**, and it covers creation steps (nonexistence by derived target URL) as well as updates — so a drift discovered late cannot produce a half-applied proposal. The dispatched `If-Match` is asserted to equal the *reviewed* baseline, which is what makes the digest meaningful rather than decorative. - **R09 is demonstrated exactly as specified:** statuses `[applied, failed, pending, pending]` with the proposal `partial`, the parent `needs_reconciliation`, children linked by `(parent, step_id)`, exactly two `PUT`s and **zero `DELETE`s**, and the applied step's write still on the backend. No rollback, no retry — the failure is recorded, not undone. - **A stale proposal cannot be acted on:** a `pending` row past its window is *reported* as expired (a computed view rather than a stored lie), so the state cannot be stale in the one place that matters. - **Reject never touches the calendar** — no CalDAV client is constructed on that path at all, which is stronger than asserting no request was made — and re-reject/apply yields `proposal_not_applicable`. - Approval is treated as audit evidence (`reviewed_at` + verified digest recorded), consistent with the contract's position that the authorized client, not the service, carries responsibility for review. Merging. `#28` (outcomes) and `#31` (agent skill) are now unblocked.

yuris merged commit f953b2d66d into main 2026-09-23 03:01:03 +00:00

yuris referenced this pull request from a commit 2026-09-23 03:01:03 +00:00

feat(api): proposal approval verification and ordered application (#70)

yuris changed title from feat(api): proposal approval verification and ordered application (#27) to feat(api): proposal approval verification and ordered application 2026-09-23 05:12:00 +00:00

yuris referenced this pull request from a commit 2026-09-23 11:54:43 +00:00

feat(api): proposal approval verification and ordered application (#70)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

yuris/life-service!70

No description provided.