docs(wiki): update project wiki

docs/wiki/Architecture Overview.md

@@ -1,13 +1,15 @@
 ---
 type: architecture
-last-updated: "2026-09-04"
-updated-by: "pr-1527"
+last-updated: "2026-09-07"
+updated-by: "schedule-d7190410-34062354146"
 sources:
   - src/main.ts
   - src/post.ts
   - src/harness/run.ts
   - src/harness/post.ts
+  - src/harness/config/state-keys.ts
   - src/services/cache/save.ts
+  - src/shared/cache-save-result.ts
   - packages/runtime/src/index.ts
   - packages/runtime/src/coordination/types.ts
   - packages/harness/src/cli.ts
@@ -132,13 +134,13 @@ The runtime package exports five module groups:
 
 ### Action Modules (`src/`)
 
-**Shared** — `logger.ts` provides JSON-structured logging with automatic credential redaction. `types.ts` defines core interfaces (`ActionInputs`, `CacheResult`, `RunContext`). `constants.ts` pins default versions for OpenCode, Bun, oMo, and Systematic.
+**Shared** — `logger.ts` provides JSON-structured logging with automatic credential redaction. `types.ts` defines core interfaces (`ActionInputs`, `CacheResult`, `RunContext`). `constants.ts` pins default versions for OpenCode, Bun, oMo, and Systematic. `cache-save-result.ts` defines the persistence-outcome contract that cleanup hands to the post-action hook and that the `cache-save-result` action output reports (see [[Session Persistence]]).
 
 **Services** — `github/` wraps Octokit and the `NormalizedEvent` system (see [[Execution Lifecycle]]). `cache/` manages GitHub Actions cache with corruption detection and S3 fallback. `setup/` orchestrates tool installation, including the new opt-in oMo installation controlled by the `enable-omo` input (see [[Setup and Configuration]]).
 
 **Features** — `agent/` bridges the runtime prompt builder with GitHub-specific context and the output-mode resolver (see [[Prompt Architecture]]). `triggers/` implements event routing and skip-condition logic, including the PR review opt-out label handled in `skip-conditions-pr.ts` (see [[Execution Lifecycle]]). `comments/` and `reviews/` handle GitHub comment and PR review posting. `context/` hydrates issue/PR data via GraphQL. `observability/` collects metrics and generates run summaries. `attachments/` processes file attachments. `delegated/` (RFC-010) manages branch, commit, and PR operations via the GitHub Git Data API, and hosts the **brokered push** state machine that commits an agent's fix directly to a same-repo PR head branch on a trusted comment trigger, without ever handing the agent a credential (see [[Execution Lifecycle]] and [[Setup and Configuration]]).
 
-**Harness** — `run.ts` orchestrates the full execution lifecycle through discrete phases, including the new lock acquisition phase. `post.ts` handles the post-action cache save. `config/` parses action inputs and manages state keys.
+**Harness** — `run.ts` orchestrates the full execution lifecycle through discrete phases, including the new lock acquisition phase. `post.ts` handles the post-action cache save, gated on the persistence outcome and metadata flags that cleanup wrote to action state. `config/` parses action inputs, publishes outputs, and manages the state keys that cross the main-step/post-hook process boundary (see [[Execution Lifecycle]]).
 
 ## Design Decisions
 

docs/wiki/Conventions and Patterns.md

@@ -1,9 +1,11 @@
 ---
 type: convention
-last-updated: "2026-08-30"
-updated-by: "schedule-d7190410-33338713321"
+last-updated: "2026-09-07"
+updated-by: "schedule-d7190410-34062354146"
 sources:
   - AGENTS.md
+  - src/shared/cache-save-result.ts
+  - src/services/cache/checkpoint.ts
   - scripts/module-taxonomy.test.ts
   - scripts/harness-tag-derivation.test.ts
   - packages/runtime/src/shared/logger.ts
@@ -65,6 +67,16 @@ Recoverable errors use `Result<T, E>` from `@bfra.me/es` rather than exceptions.
 
 Exceptions are reserved for truly unexpected failures (programmer errors, system failures). SDK operations in the session module take this further: they return empty arrays or `null` on failure, never throwing.
 
+## Named Outcomes Over Booleans
+
+A related habit runs through the operations-facing parts of the codebase: when an operation can end several materially different ways, it returns a named terminal condition rather than a boolean. `CheckpointOutcome` in `src/services/cache/checkpoint.ts` and `CacheSaveResult` in `src/shared/cache-save-result.ts` are the reference examples.
+
+The motivation is consistent. A boolean forces "deliberately skipped", "denied by the platform", and "succeeded" onto two poles, which destroys the distinction exactly where an operator most needs it — in a job summary or an action output, after the fact, with no way to re-observe the run. The replacement typically pairs one or more structural axes with a named outcome: `CacheSaveResult` carries `cachePersisted` and `storePersisted` as independent facts, plus an `outcome` naming the condition that produced them.
+
+Two supporting practices travel with the pattern. Mappings from an outcome union to some other representation are pinned with `satisfies Record<Outcome, ...>`, so adding a new outcome without handling it fails `check-types` rather than silently mapping to nothing. And where a value crosses a process boundary and is read back as a string — the action-state handoff between the main step and the post hook — parsing is defensive in a stated direction: an absent or unrecognized value maps to the outcome that causes the work to be retried, never the one that causes it to be skipped, because the reader is the last chance to do the work.
+
+Comments in these modules also distinguish inference from observation. `cache-save-result.ts` states explicitly that `cache-rejected` covers several causes because the API boundary cannot tell them apart, not because one was determined over the others — and warns against inferring the cause from surrounding signals. Documenting the limits of what a value proves is treated as part of defining it.
+
 ## NormalizedEvent
 
 Raw GitHub webhook payloads are never accessed directly anywhere in the codebase. Every payload passes through `normalizeEvent()` in `src/services/github/context.ts`, which produces a `NormalizedEvent` — a discriminated union with eight variants:

docs/wiki/Execution Lifecycle.md

@@ -1,8 +1,11 @@
 ---
 type: architecture
-last-updated: "2026-09-04"
-updated-by: "pr-1536"
+last-updated: "2026-09-07"
+updated-by: "schedule-d7190410-34062354146"
 sources:
+  - src/harness/config/state-keys.ts
+  - src/shared/cache-save-result.ts
+  - src/features/observability/job-summary.ts
   - src/harness/run.ts
   - src/harness/phases/bootstrap.ts
   - src/harness/phases/routing.ts
@@ -169,7 +172,17 @@ The cleanup phase has its own `finally` block for lock release: if a coordinatio
 
 ## Post-Action Hook
 
-`post.ts` runs after the main step completes — even if the main step was cancelled or failed. It exists because GitHub Actions may kill the main step's `finally` block after a brief grace period, which could interrupt the cache save. The post-action hook provides a second, durable opportunity to persist state. It reads flags from action state to determine whether the main step already saved successfully, avoiding redundant work.
+`post.ts` runs after the main step completes — even if the main step was cancelled or failed. It exists because GitHub Actions may kill the main step's `finally` block after a brief grace period, which could interrupt the cache save. The post-action hook provides a second, durable opportunity to persist state. It reads state written by cleanup to decide what still needs doing.
+
+### The Cleanup-to-Post Handoff
+
+Two state keys cross the process boundary between the main step and the post hook (`src/harness/config/state-keys.ts`), and they answer different questions.
+
+`CACHE_SAVED` carries a four-value enum rather than a boolean — `durable`, `store-only`, `skipped`, or `not-persisted` — derived from cleanup's `CacheSaveResult` (see [[Session Persistence]]). The boolean it replaced conflated a store-only save with total failure, so the post hook re-ran a save whose object-store upload had already landed, uploading the same content twice. The enum lets the hook gate on durability actually achieved rather than on cache-write success alone. `durable` and `store-only` skip; `skipped` skips because the operator opted out. Everything else retries — including a state value that is absent or unrecognized, which is deliberately mapped to `not-persisted` on the reasoning that the post hook is the last chance to persist and a garbled value must fail toward doing the work.
+
+`skipped-empty` is worth calling out as the case that does _not_ fold into `skipped`. "No cacheable content existed" is a point-in-time filesystem observation, not a configuration constant, so the post hook retries it: the retry re-runs the SQLite checkpoint before re-checking for content, a small cost weighed against losing a session.
+
+`CLEANUP_METADATA_WRITTEN` answers a question `CACHE_SAVED` structurally cannot. Cleanup uploads a rich run-metadata payload — token usage, timing, session IDs, PRs and commits, errors — to the object store, and the post hook has only a thin `cleanupSkipped: true` placeholder to write. But `run.ts` seeds `CACHE_SAVED` with `not-persisted` before cleanup ever executes, so that value is equally consistent with "cleanup ran and its cache write was denied" and "cleanup never ran at all." Using it to gate the metadata write meant the placeholder could clobber the real payload. The dedicated key is set unconditionally by cleanup, independent of the cache outcome, and the post hook writes its placeholder only when the key is absent.
 
 ## Event Types
 

docs/wiki/Prompt Architecture.md

@@ -1,9 +1,10 @@
 ---
 type: subsystem
-last-updated: "2026-08-30"
-updated-by: "schedule-d7190410-33338713321"
+last-updated: "2026-09-07"
+updated-by: "schedule-d7190410-34062354146"
 sources:
   - packages/runtime/src/agent/prompt.ts
+  - packages/runtime/src/agent/response-file.ts
   - packages/runtime/src/agent/prompt-thread.ts
   - src/features/agent/prompt-sender.ts
   - packages/runtime/src/agent/output-mode.ts
@@ -95,6 +96,18 @@ How the agent is told to deliver its answer is decided in bootstrap by `resolveR
 
 Keeping this branch in the prompt builder (rather than in a separate agent instruction file) means the response protocol the model sees always matches what the harness will actually do with its output, so the two can never drift.
 
+## Response Surfaces
+
+Delivery mode answers "how does the answer get posted." A second, orthogonal axis answers "what kind of artifact is this run allowed to produce," and it is carried by `ResponseSurface` (`packages/runtime/src/agent/response-file.ts`). Four surfaces exist: `issue-comment`, `pr-comment`, `pr-review`, and `pr-review-permitted`.
+
+The interesting one is `pr-review-permitted`, which exists because an authorized `@fro-bot` mention on a pull request does not fit either of the two states that preceded it. A `pr-review` run must produce a verdict — that is what the trigger asked for. A `pr-comment` run must not. A mention on a PR is legitimately either: the human may be asking for a review, or may be asking a question that deserves a prose answer. The two-state surface forced a choice between rejecting every mention verdict and demanding one from every PR mention. On the permitted surface a verdict is accepted and submits a real review through the existing fork and head-SHA guards, and its absence is equally valid and posts a comment.
+
+Surface is derived once at the trusted boundary and threaded into prompt building as a required field, so no caller can re-derive it differently. That matters because the surface is what makes the response protocol section coherent: an earlier version instructed every file-convention run to emit a verdict regardless of surface, which meant an issue-surface run was explicitly told to do the one thing its own validator would reject.
+
+The policy for each surface lives in one total table (`RESPONSE_SURFACE_POLICIES`), giving each surface a `target`, and three independent booleans: `verdictRequired`, `verdictPermitted`, and `reviewCapable`. The table is pinned with `satisfies Record<ResponseSurface, ResponseSurfacePolicy>` because widening the union alone does not make the type checker enumerate its consumers — before the table, each consumer branched with equality checks, so a newly added surface would compile cleanly and silently fall through to comment delivery. This is the same exhaustiveness discipline described in [[Conventions and Patterns]].
+
+Mismatches degrade asymmetrically, in the direction that preserves work. A verdict arriving on a comment surface drops the frontmatter and posts the prose with a warning rather than discarding the turn; a missing verdict where one is genuinely required still fails closed.
+
 ## Prompt Sender
 
 The assembled prompt text and reference files are sent to an OpenCode SDK session by `sendPromptToSession()` in `src/features/agent/prompt-sender.ts`. (This module lives in the action layer — the runtime no longer carries a parallel prompt-sender after the execution stack was consolidated; see [[Architecture Overview]].) The function handles model resolution (if a model override is configured), directory scoping to the GitHub workspace, and the construction of the SDK message payload with both text and file parts. For retry attempts after LLM failures, a short continuation prompt is sent instead of the full initial prompt, since the session already has the full context. That continuation text is built from the observed error type rather than a fixed string, and asks the model to continue the remaining objective rather than blindly resume — the [[Execution Lifecycle]] covers how the attempt outcome is classified and how the continuation is phrased.

docs/wiki/Session Persistence.md

@@ -1,8 +1,10 @@
 ---
 type: subsystem
-last-updated: "2026-09-04"
-updated-by: "pr-1534"
+last-updated: "2026-09-07"
+updated-by: "schedule-d7190410-34062354146"
 sources:
+  - src/services/cache/cache-key.ts
+  - src/shared/cache-save-result.ts
   - packages/runtime/src/session/storage.ts
   - packages/runtime/src/session/search.ts
   - packages/runtime/src/session/prune.ts
@@ -49,6 +51,8 @@ opencode-storage-{repo}-{branch}-{os}
 
 Branch scoping prevents one branch's session history from leaking into another. The OS component handles the rare case of cross-platform runners.
 
+Restore and save use that key differently. Restore matches on two prefixes — branch-scoped first, then repo-scoped — while the key a run _writes_ appends its run ID and run attempt (`buildSaveCacheKey` in `src/services/cache/cache-key.ts`). The run-attempt component was added because Actions cache entries are immutable: a re-run keeps the same `GITHUB_RUN_ID`, so without the attempt suffix the second attempt's save collided with the first attempt's entry, and that collision was folded into the "already exists" success path — the retry's own session state was silently discarded while the save reported success. Each attempt now writes its own entry. Both restore prefixes stop before the run ID, so they continue to match every save key regardless of run or attempt; the cost is that re-runs consume additional entries against the repository's cache budget, which LRU eviction absorbs. Losing a re-run's session state was judged the worse outcome.
+
 On restore, the cache module performs several safety checks:
 
 - **Corruption detection** — Verifies the storage path is a readable directory. If not, cleans and continues with empty state.
@@ -65,7 +69,17 @@ This mapping from event to cache scope is attributed to a platform policy change
 
 The result is that a run can restore prior state successfully while its cache write is rejected. Mention-driven runs therefore read session state but may be unable to write their new state. This is a GitHub platform constraint, not a Fro Bot configuration bug. `s3-backup` defaults to `false`; enabling it with a configured bucket makes the object store the durable path for session state, and restore consults it before the Actions cache. Object-store sync is best-effort and non-fatal, so continuity depends on the upload succeeding. If the upload fails, the run still falls through to the Actions cache write, which may also be denied.
 
-`@actions/cache` hides both write denials and reservation collisions behind a `-1` sentinel instead of throwing, so the save path cannot distinguish those causes at that boundary. The cache write is therefore reported as not persisted rather than as a confident diagnosis. The post-action hook remains the retry path when the main-step save did not persist.
+`@actions/cache` hides both write denials and reservation collisions behind a `-1` sentinel instead of throwing, so the save path cannot distinguish those causes at that boundary. The cache write is therefore reported as not persisted rather than as a confident diagnosis. The post-action hook remains the retry path when the main-step save did not persist. What "did not persist" means precisely is the subject of the next section.
+
+### What a Save Reports
+
+`saveCache` returns a structured `CacheSaveResult` (`src/shared/cache-save-result.ts`) rather than a boolean. The shape has two independent persistence axes — `cachePersisted` and `storePersisted` — plus a named `outcome` describing the terminal condition that produced them. Durability overall is the disjunction of the two axes, never either one alone.
+
+The reason for the shape is that a boolean collapsed three genuinely different situations into the same value: a deliberate opt-out (`SKIP_CACHE`), a denied write, and a successful write. Naming them separately makes the run's actual persistence story legible in the job summary and in the `cache-save-result` action output. The outcomes are `skipped-by-configuration`, `skipped-empty`, `checkpoint-declined`, `cache-rejected`, `cache-error`, and `persisted`.
+
+`cache-rejected` deserves a note about what it does _not_ claim. `@actions/cache` distinguishes a policy denial, a reservation collision, a finalize error, a 5xx, and an upload failure internally, but none of those survive the `saveCache()` call boundary — every one returns the same `-1`. `cache-rejected` therefore covers all of them because the boundary cannot tell them apart, not because one was determined over the others. The source comments are explicit that this is an inference and that the cause must not be guessed from the trigger type or runner configuration: a self-hosted runner can hold a writable token on a comment trigger, and a transient service failure can happen regardless of token permissions.
+
+The `persisted` outcome absorbs one case that looks like a failure. When the cache adapter throws an "already exists" error, the key is durably written either way — some concurrent job committed it first — so the state is present under that key regardless of which run wrote it. That is folded into `persisted` rather than given its own outcome.
 
 Before saving, `saveCache` in `src/services/cache/save.ts` checks whether there is anything worth caching — but this check is subtler than "is the storage directory non-empty." Recent OpenCode versions persist sessions in an `opencode.db` SQLite file in the _parent_ of the storage directory, not inside it, so a naive empty-directory check would skip the save on every real run. The save path checkpoints SQLite before building the transport paths, and today's cache and object-store save sets include `opencode.db` but not `opencode.db-wal` or `opencode.db-shm`. A non-empty `opencode.db` is therefore the DB-side signal; WAL-only data is expected to have been merged before the save check.