fix: close final loop review boundaries

Author: codeacme17

.codex/agents/echo-ui-pr-reviewer.toml

@@ -10,9 +10,10 @@ security, regressions, and missing tests. Ignore executor conversation history a
 do not modify files. Avoid style-only or speculative findings. Return a structured
 review with run-wide `RVW-<cycle>-<round>-<sequence>` IDs, severity P0-P3, confidence, file and line, concrete
 evidence, reproduction when possible, and expected resolution. Return PASS when no
-actionable findings exist. Publish GitHub reviews only through the executable
-`loops/issue-dev-loop/scripts/with-github-identity reviewer -- ...` launcher; the
-wrapper must verify the configured reviewerGitHubLogin before publication. Never
+actionable findings exist. Publish GitHub reviews only through the installed executable
+`"$ECHO_UI_LOOP_CONTROL_PLANE/scripts/with-github-identity" --loop-root
+"$ECHO_UI_LOOP_TARGET_ROOT" reviewer -- ...` launcher; the wrapper must verify the
+configured reviewerGitHubLogin before publication. Never use the repository launcher,
 invoke the `.mjs` implementation directly, run raw `gh`, alter global authentication,
 or let the executor identity publish on your behalf. Use `gh pr review --comment --body`
 for a body-only review; body files are prohibited. When a finding has a file and line, use the launcher's

.codex/agents/echo-ui-review-adjudicator.toml

@@ -9,8 +9,9 @@ from private conversation history. Return ACCEPT_FINDING, REJECT_FINDING, or
 NEEDS_OWNER, followed by concise evidence. Prefer NEEDS_OWNER when product intent,
 public API policy, security, or release policy is ambiguous.
 For REJECT_FINDING, publish the required adjudication marker only through the
-executable `loops/issue-dev-loop/scripts/with-github-identity reviewer -- ...`
-launcher. Use a non-approving `gh pr review --comment`. Do not invoke the `.mjs`
-implementation directly or run raw or mutating `gh api`,
+installed executable `"$ECHO_UI_LOOP_CONTROL_PLANE/scripts/with-github-identity"
+--loop-root "$ECHO_UI_LOOP_TARGET_ROOT" reviewer -- ...` launcher. Use a non-approving
+`gh pr review --comment`. Do not use the repository launcher, invoke the `.mjs`
+implementation directly, or run raw or mutating `gh api`,
 alter global authentication, or post through the executor identity.
 """

loops/_shared/owner-channel/CHANNEL.md

@@ -8,7 +8,7 @@ GitHub issue and PR comments are the canonical, auditable channel. The runtime m
 
 `--dry-run` stages a simulated payload only. It never records `owner_notified`, never pauses the run, and never satisfies a blocking delivery gate.
 
-`pr_completed` is immediate but informational and must remain non-blocking. After remotely observing the owner-authored Ready transition, exact-head owner approval, and owner merge, the runtime durably posts this GitHub notification and performs the same bounded optional webhook attempt before it records the local merge/final state.
+`pr_completed` is informational and does not ask the owner for another decision, but successful canonical GitHub delivery is a hard prerequisite for completion. `prepare-finalization` posts it only after remotely observing the owner-authored Ready transition, exact-head owner approval, and owner merge; it waits for the bounded optional webhook attempt and binds both notification outcomes before any terminal journal publication. GitHub delivery failure leaves the run active and retryable.
 
 Each blocking GitHub notification prints a unique resume instruction. To continue after answering, include `RESUME <run-id>` in a normal issue/PR comment; submitting a GitHub `CHANGES_REQUESTED` review is also an explicit response. The runtime verifies author, target, timestamp, successful delivery, and response URL before resuming. Silence and unrelated comments never count.
 

loops/issue-dev-loop/LOOP.md

@@ -103,7 +103,7 @@ When the owner requests changes, verify the owner-authored GitHub comment or `CH
 
 ### 10. Complete
 
-Only the remote owner-merge gate permits `completed`. Both finalization publication and the terminal transition independently query GitHub and must observe the recorded issue branch still targeting `dev`, an `APPROVED` review by `codeacme17` for the exact reviewed head SHA, and a merge performed by `codeacme17` for that same head. Mutable local events are audit records, never authorization. Before any terminal transition, generate a canonical finalization record, publish it through the automation identity to the configured GitHub state-journal issue, and validate its comment URL and digest. Reconciliation revalidates completed records remotely before accepting them or suppressing an active checkpoint. Record merge SHA and timestamp, remove `loop:claimed`, update state, append the run summary, and retain links to published evidence. A closed unmerged PR is `cancelled`, not completed.
+Only the remote owner-merge gate permits `completed`. Before a completion record exists, `prepare-finalization` remotely verifies the automation-authored Ready notification, an owner `ready_for_review` timeline event after that comment with no later redraft, an `APPROVED` review for the exact reviewed head SHA, and a merge performed by `codeacme17`. It then requires successful canonical `pr_completed` GitHub delivery and waits for the optional webhook attempt to settle within its bound. The finalization record binds the Ready and completion comment URLs/timestamps plus webhook outcome. Only after those prerequisites may the automation identity publish the record to the GitHub state journal. Publication, `observe-owner-merge`, terminal transition, and future reconciliation re-fetch that proof; a failed delivery or crash before publication leaves the active checkpoint resumable instead of suppressing it. Mutable local events are audit records, never authorization. Record merge SHA and timestamp, remove `loop:claimed`, update state, append the run summary, and retain links to published evidence. A closed unmerged PR is `cancelled`, not completed.
 
 ## State and history
 

loops/issue-dev-loop/SKILL.md

@@ -61,7 +61,7 @@ Run verification appropriate to the change and require `pnpm verify` before the
 
 Before publishing the final review round, write the complete cycle result with an unassigned final `reviewUrl`, then run `loopctl.mjs review-digest --result <absolute-path>`. Add the returned marker to the final review body, publish the non-approving review, replace the unassigned URL with GitHub's actual review URL, and confirm `review-digest` is unchanged. After all responses are posted, run `loopctl.mjs record-review --run-id <id> --result <absolute-path> --review-url <github-review-url>`. The publication digest deliberately canonicalizes GitHub-assigned review URLs while the stored full-file digest still protects the final artifact. Both evidence and review gates must name the current PR head. Keep the PR Draft, emit a blocking `pr_ready_for_review` notification asking `codeacme17` to mark it Ready and review it, then transition from `waiting_for_owner` to `awaiting_owner_review` with the PR URL and exact head SHA.
 
-The owner is the only actor allowed to mark a Draft PR Ready, approve it, or merge it. Never call non-`--undo` `gh pr ready`, `gh pr merge`, enable auto-merge, push to `main`, push to `dev`, dismiss owner feedback, or bypass branch protections. Before any terminal transition, run `prepare-finalization`, publish its exact body to the configured state-journal issue, and validate the returned comment URL with `record-finalization`. A completed run passes the same result and comment URL to `observe-owner-merge`, which paginates the PR timeline and reviews, requires a post-notification Ready transition authored by `codeacme17` with no later redraft, and requires the owner's exact-head approval and merge. It then delivers the informational `pr_completed` GitHub/webhook notification before recording the merge and terminal state. Future workspaces run `reconcile` to rebuild local history and evolve metrics from those automation-authored journal comments.
+The owner is the only actor allowed to mark a Draft PR Ready, approve it, or merge it. Never call non-`--undo` `gh pr ready`, `gh pr merge`, enable auto-merge, push to `main`, push to `dev`, dismiss owner feedback, or bypass branch protections. Before any terminal transition, run `prepare-finalization`. For completion, that command paginates the PR timeline and reviews, requires a Ready transition authored by `codeacme17` after the remotely verified Ready-notification comment with no later redraft, requires the owner's exact-head approval and merge, delivers the informational `pr_completed` GitHub notification, waits for the bounded webhook attempt to settle, and binds both notification URLs/timestamps into the record. If canonical GitHub delivery fails, no terminal record is created. Only then publish the exact body to the configured state-journal issue and pass its result/comment URL to `observe-owner-merge`; that command revalidates the remote record, appends the local notification/owner audit events, and finalizes. Future workspaces run `reconcile` to revalidate the same notification, Ready, approval, and merge proof before rebuilding local history or suppressing an active checkpoint.
 
 For any pause, do not resume from silence or an arbitrary comment. First require a successfully delivered blocking notification. Then verify the owner's GitHub decision with `loopctl.mjs record-owner-response --run-id <id> --response-url <comment-or-review-url>`. A normal comment must include the exact `RESUME <run-id>` token printed in the notification; a `CHANGES_REQUESTED` review is itself an explicit decision and must be submitted against the run's current exact head SHA. Only then may `loopctl.mjs transition --run-id <id> --status running` continue. Before any repair work or new push, publish that transition's checkpoint, run the unchanged exact PR through `gh pr ready --undo --repo codeacme17/echo-ui`, observe the same head as Draft with `record-pr`, and publish another checkpoint. `$implement` repair attestations and later PR rebinds are rejected unless this durable redraft happened after the current owner response.
 

loops/issue-dev-loop/dependencies.md

@@ -41,6 +41,6 @@ The installer refuses a dirty checkout, a branch other than `dev`, a commit othe
 
 The trust root is the scheduler/OS boundary, not Unix mode bits or a self-hashed manifest. Put the installed bundle outside every filesystem root writable by the unattended Codex process, and expose it read/execute-only through the scheduler sandbox or separate ownership/ACLs. The automation identity must not have permission to change the bundle, its parent, the pinned executables, file modes, ACLs, or the sandbox policy. A same-OS-principal process allowed to rewrite both code and its manifest cannot establish a cryptographic trust root in user space; do not activate the loop under that permission model.
 
-Credential profiles are a separate secret boundary. Put both profile directories outside every root listed by `ECHO_UI_LOOP_UNTRUSTED_ROOTS`, set each directory to mode `0700` and every entry below it to deny group/other access, and expose them only to the trusted orchestration/identity-router process. The activation check canonicalizes these paths, rejects symlinks, broad permissions, wrong ownership, missing repository coverage, profiles inside an untrusted root, and overlapping agent-visible roots. `$implement`, fresh reviewers, candidate scripts, local product tests, and verifier containers must receive neither profile-path variables nor `GH_CONFIG_DIR`, and their sandbox must not be able to read the profile directories. If the scheduler cannot enforce that read boundary, do not activate the loop.
+Credential profiles are a separate secret boundary. Put both profile directories outside every root listed by `ECHO_UI_LOOP_UNTRUSTED_ROOTS`, set each directory to mode `0700` and every entry below it to deny group/other access, and expose them only to the trusted orchestration/identity-router process. The activation check rejects the originally configured path when it is a symlink, then canonicalizes the real directory, checks descendants, permissions, ownership, repository coverage, and overlap with agent-visible roots, and routes `GH_CONFIG_DIR` through that validated canonical path. `$implement`, fresh reviewers, candidate scripts, local product tests, and verifier containers must receive neither profile-path variables nor `GH_CONFIG_DIR`, and their sandbox must not be able to read the profile directories. If the scheduler cannot enforce that read boundary, do not activate the loop.
 
 Never run `gh auth setup-git` for this loop. Route commands through `"$ECHO_UI_LOOP_CONTROL_PLANE/scripts/with-github-identity" --loop-root "$ECHO_UI_LOOP_TARGET_ROOT" <role> -- ...`. The repository copy is installer source and intentionally refuses credential use. The installed launcher verifies its manifest before selecting an identity, removes Node preload hooks, scopes `GH_CONFIG_DIR` and the Git credential helper to one allowlisted child tree, gates descendant `git`/`gh` calls, and leaves the user's default `gh` account and global Git credential configuration unchanged.

loops/issue-dev-loop/references/github-operations.md

@@ -42,7 +42,7 @@ The shell launcher removes Node preload hooks before starting the router. The ro
 
 ## Evidence artifact
 
-The low-privilege `pull_request` workflow `Issue dev loop evidence` runs only when the branch contains one active issue run. All checkouts disable persisted credentials. It resolves the run's frozen base from the exact candidate, proves that base remains an ancestor of live `dev`, and checks out that immutable owner-merged commit separately from the current control checkout. Container preparation installs the protected candidate and frozen-baseline lockfiles with lifecycle scripts disabled into independent Docker volumes before candidate code runs. Candidate `pnpm verify` and the actual frozen owner-merged baseline `pnpm test` run with `--network none`, no inherited GitHub token, and no mount of any host checkout. The frozen-base generator remains outside those containers and binds the manifest to candidate head, workflow-run SHA, frozen owner-merged base SHA, and the live PR base SHA. The installed control plane later rejects the artifact unless its own exact-head diff check proves that the PR did not change the loop plane, owner channel, workflow, verification scripts, package manifests, lock/workspace files, package hooks, or verification configuration. Wait for that exact-head run to complete, then locate and download the artifact:
+The low-privilege `pull_request` workflow `Issue dev loop evidence` runs only when the branch contains one active issue run. All checkouts disable persisted credentials. It resolves the run's frozen base from the exact candidate, proves that base remains an ancestor of live `dev`, and checks out that immutable owner-merged commit separately from the current control checkout. Container preparation installs the protected candidate and frozen-baseline lockfiles with lifecycle scripts disabled into independent Docker volumes before candidate code runs. Candidate `pnpm verify` and the actual frozen owner-merged baseline `pnpm test` run with `--network none`, no inherited GitHub token, and no mount of any host checkout. The frozen-base generator remains outside those containers and binds the manifest to candidate head, workflow-run SHA, frozen owner-merged base SHA, and the live PR base SHA. The installed control plane later rejects the artifact unless its own NUL-safe, rename-disabled exact-head diff check proves that the PR did not change either side of a loop/control rename, the owner channel, workflow, agent/prompt configuration, deployment-provider configuration, verification scripts, package manifests, lock/workspace files, package hooks, or verification configuration. Exact protected root entries and symlinks are rejected as well as descendants. Wait for that exact-head run to complete, then locate and download the artifact:
 
 ```text
 "$ECHO_UI_LOOP_CONTROL_PLANE/scripts/with-github-identity" --loop-root "$ECHO_UI_LOOP_TARGET_ROOT" automation -- gh run list --workflow issue-dev-loop-evidence.yml --branch codex/issue-<number>
@@ -77,4 +77,4 @@ After a blocking notification, verify the reply URL with `record-owner-response`
 
 For active work, run `loopctl prepare-checkpoint --run-id <id>`, post its exact `body` to the configured `stateIssueNumber` using the automation identity, then validate it with `loopctl record-checkpoint --run-id <id> --result <path> --comment-url <url>`. A checkpoint is SHA-256 bound to the active run, frozen brief, ordered validated events, and the small local result/manifest artifacts required by later gates. Restore verifies every embedded artifact digest before recreating it. Publish one after every state-changing phase; later checkpoints supersede earlier ones.
 
-Before `completed`, `failed`, `blocked`, or `cancelled`, run `loopctl prepare-finalization` with the terminal status and any merge SHA/failure fingerprint. Failed/blocked records bind the delivered automation-authored owner-notification URL; completion binds and remotely rechecks owner approval and merge; cancellation requires the recorded PR to be closed without merge. Post the exact `body` to the configured `stateIssueNumber` using the automation identity, then run `loopctl record-finalization --run-id <id> --result <path> --comment-url <url>`. For completion, pass that result and URL to `observe-owner-merge`. Terminal transitions reject missing, edited, wrong-author, wrong-issue, digest-mismatched, or externally unproven journal entries. Every scheduled wake begins with `loopctl reconcile`, which restores missing local history and recomputes evolve metrics from the append-only journal.
+Before `completed`, `failed`, `blocked`, or `cancelled`, run `loopctl prepare-finalization` with the terminal status and any merge SHA/failure fingerprint. Failed/blocked records bind the delivered automation-authored owner-notification URL; cancellation requires the recorded PR to be closed without merge. Completion preparation first re-fetches the Ready-notification comment, requires the owner's later Ready event, exact-head approval, and merge, delivers `pr_completed`, and waits for the bounded webhook attempt. It refuses to create a terminal record if canonical GitHub delivery fails. The record binds both notification URLs/timestamps and the webhook outcome so reconciliation cannot silently skip completion delivery. Post the resulting exact `body` to the configured `stateIssueNumber` using the automation identity, then pass that result and comment URL to `observe-owner-merge`; it revalidates the journal entry before recording local audit events and finalizing. Terminal transitions reject missing, edited, wrong-author, wrong-issue, digest-mismatched, or externally unproven journal entries. Every scheduled wake begins with `loopctl reconcile`, which restores missing local history and recomputes evolve metrics from the append-only journal.

loops/issue-dev-loop/schemas/finalization-record.schema.json

@@ -13,7 +13,11 @@
     "headSha",
     "mergeSha",
     "failureFingerprint",
-    "notificationUrl"
+    "notificationUrl",
+    "readyNotificationUrl",
+    "readyNotifiedAt",
+    "completionNotifiedAt",
+    "notificationWebhookStatus"
   ],
   "additionalProperties": false,
   "properties": {
@@ -33,6 +37,10 @@
       "pattern": "^[0-9a-fA-F]{40}$"
     },
     "failureFingerprint": { "type": ["string", "null"] },
-    "notificationUrl": { "type": ["string", "null"], "format": "uri" }
+    "notificationUrl": { "type": ["string", "null"], "format": "uri" },
+    "readyNotificationUrl": { "type": ["string", "null"], "format": "uri" },
+    "readyNotifiedAt": { "type": ["string", "null"], "format": "date-time" },
+    "completionNotifiedAt": { "type": ["string", "null"], "format": "date-time" },
+    "notificationWebhookStatus": { "type": ["string", "null"] }
   }
 }