[CR-7][P1] Add explicit review actions and head-guarded GitHub merge

Outcome

Make the screenshot's comment/review actions and Merge control functional without confusing model output, cached UI state or repeated clicks with user authorization. Every external write is a deliberate, account/repository-scoped action with a visible outcome. Unknown outcomes are reconciled, not blindly retried.

Parent: #319. Blocked by accepted artifacts: #321's authenticated read/identity service, #323's overview/action slots and #324's revision/line anchors. #325's AI findings are optional draft input, NOT a dependency or authority source. This issue authorizes implementation and mock tests; it does not authorize merging existing PRs or posting test reviews to production repositories.

Context / entry points

Read current AGENTS.md, current main/open PRs, docs/status.md and #320's actual source/contract map. Audit baseline cb33db5df00e235f68d36b5fac556dc30e0851f2 is historical evidence, not a base to force onto later work. Reuse existing validated IPC, identity and durable-receipt mechanisms only where their semantics fit external writes. Native tool approval from #276 is not equivalent to GitHub permission.

Proposed additions if no suitable equivalent exists: apps/desktop/src/main/engine/code-review/write-service.ts, focused draft/intent models in the accepted shared contract, apps/desktop/test/code-review-writes.test.ts and apps/desktop/e2e/code-review-actions.spec.ts. Avoid a generic command bus, new orchestration framework or separate auth system. Deliver draft/submit protection and merge protection as separately reviewable increments within this issue.

Items to do

Write failing tests for explicit write intents scoped to current actor/account, stable repository, PR, operation, payload and reviewed head. Main validates sender/session and resolves identity/capabilities immediately before execution; a renderer-provided canWrite boolean is not authority.
Implement account/PR-scoped local drafts and an explicit preview/submit flow for a top-level comment or submitted review. Inline review comments, when enabled, use [CR-5][P1] Add revision-bound remote diffs, context folding and file navigation #324's validated path/side/line/revision anchors. Preserve drafts after errors; never send merely because a PR was selected or an AI finding appeared.
Wire title/body edits and reviewer requests only if included by [CR-1][P1] Establish the Code Review reference contract and deterministic fixtures #320's accepted control inventory, through the same bounded validation/confirmation path. Show missing permission, self-review restrictions or unsupported controls honestly. Do not add delete, admin, force-push or stack-mutation actions.
Show the exact destination, account and requested action before a consequential submit. Review/merge confirmations include the reviewed head and target branch; reject stale or reused confirmation intents. Changed account/head/target, revoked permission and unresolved state require refresh/reconfirmation, not implicit adaptation.
Implement merge preflight from fresh GitHub state and server-supported capabilities: PR state/draft status, head, base, permitted merge methods and policy/mergeability information. Keep unknown/blocking states unavailable and offer Open in GitHub when rules or queues cannot be safely supported. Never infer readiness from a green count or AI verdict and never offer a rules bypass.
Send the expected head SHA with a supported merge request and let GitHub enforce its rules. Revalidate a changed base/target before submission. Document that expected-head protection alone is not an atomic base-SHA lock; do not promise guarantees the endpoint does not provide. If the product requires stricter atomic policy than the API supports, leave that path unavailable instead of inventing client-side certainty. No automatic merge queue enrollment, auto-merge or chained stack merges.
Prevent duplicate local attempts with one in-flight intent and bounded, durable receipts. Distinguish prepared, dispatched, confirmed, rejected and unknown outcomes. Double-click, reload, timeout and crash must not blindly resend POST/PUT operations. GitHub does not become exactly-once because the client has an intent ID: reconcile the actual remote state conservatively and require a new user decision when certainty is unavailable.
Reconcile the resulting comment/review/merge state with [CR-2][P1] Add a main-owned, revision-aware GitHub PR read service #321 and update the correct PR only. Keep credentials out of drafts, screenshots, Notes and logs; scope private draft storage and sign-out visibility to the correct account. Do not overwrite a newer user draft or edit during an asynchronous response.

Acceptance criteria

AC1: comments/reviews remain local until an explicit user submit; permission/identity checks occur in main and server denials are surfaced without false success.
AC2: two rapid clicks dispatch at most one request for the same live intent. Restart after an ambiguous network outcome dispatches none automatically and exposes reconciliation state.
AC3: wrong-account/repository intents, stale heads, changed targets, revoked permissions and outdated inline anchors are rejected or require explicit renewed confirmation.
AC4: merge passes the reviewed head SHA, offers only supported methods and never treats unknown checks, model findings or inaccessible policy as permission. Unsupported queue/policy cases have a safe fallback.
AC5: drafts survive recoverable errors, remain isolated across accounts/PRs and are not silently overwritten by stale responses. Raw credentials never appear in renderer/receipt/evidence output.
AC6: mock main/IPC and Electron tests cover success, rejection, timeout, crash and reconciliation. Any live write/merge canary uses a separately authorized disposable repository/PR; absent authorization leaves this validation gate open.

Validation / handoff

Run focused intent/receipt tests and Electron confirmation/recovery tests, then pnpm build && pnpm test and pnpm test:e2e. Inspect all added mutation endpoints and permissions. Record exact SHA, commands, request counts for duplicate/restart cases, server-denial handling and mock versus live evidence in docs/reviews//docs/status.md. Do not weaken branch protection, publish real test comments or merge this implementation as a test.

Sources: #319; GitHub PR merge endpoint and expected SHA, reviews, REST best practices. Verify exact current endpoint fields and principal permissions before implementation.

Long-running goal prompt

Implement CR-7 for #319 in TypeSafeAI/modex after reading the accepted #320/#321/#323/#324 contracts, current source, open PRs and scoped AGENTS.md. Verify worktree ownership and current main. Begin with failing intent, duplicate-send, stale-head and restart tests. Build a narrow main-owned write path with isolated drafts, explicit user confirmation, fresh identity/permission checks and conservative unknown-outcome recovery. Model findings are optional draft content, never authorization. Protect merge with the exact reviewed head and server rules; do not offer bypasses, silently change targets or promise an unsupported atomic base lock. Reuse suitable existing IPC/receipt machinery without inventing a workflow engine. Keep credentials out and preserve newer drafts. Use mocks by default; no real reviews or merges without separate explicit authorization for a disposable test target. Work in small independently reviewed increments, checkpoint SHA, commands/results, evidence, limits and one next action. Finish with a signed PR and clear residual live gates. Do not merge, deploy or expand permissions to clear a failure.

Follow-up / resume prompt

Resume CR-7 by reading current issue/PR state and the actual owned-worktree diff, then rerun the intent/recovery smoke tests. Reconcile existing write receipts before any operation; unknown does not mean unsent. Revalidate account, PR head, target and policy after interruption. Continue only a missing acceptance item with mock writes unless live test authority is explicit. Preserve drafts and unrelated work, and never retry a consequential operation merely because a previous session timed out. Update fresh evidence and the next action; do not merge or post to production.

Independent review prompt

Audit all mutation entry points, not just disabled buttons. Test forged/stale intents, identity switching, double-clicks, self-review/server denial, changed heads and lost responses after a successful remote write. Verify restart cannot replay an uncertain mutation and merge cannot bypass rules or misuse an AI verdict. Check draft isolation and endpoint guarantees. Require reproducible main/IPC evidence and explicit authority for any live canary.

The CR-1 contract is on main as 43d5b9c (#334). Bind to these files at that commit:

apps/desktop/src/shared/code-review.ts (the contract)
apps/desktop/test/fixtures/code-review.ts (synthetic fixtures)
docs/design/code-review.md (source map, references, inventory, open decisions)

For CR-7:

capabilities keeps comment, submit-review, request-reviewers, edit, state and merge unavailable, with this issue as the owner.
The read channel never carries a write.

Coven-Harness: Claude Code / claude-opus-5-5

Refreshed goal prompts — 2026-10-22 (`main` at `0e674e7`)

Where it stands: not started. It needs #323's action slots and #324's line anchors. capabilities() keeps comment, submit-review, request-reviewers, edit, state and merge unavailable, owned here, and the code-review:read channel never carries a write.

Needs Val before any write code:

How writes are authorized. CR-2 reads through the person's gh login. docs/status.md → Next Convention: one worktree per session under .worktrees/ #3 says repository actions would go through a dedicated GitHub App, and that hasn't been decided. Writing through gh needs Val's explicit approval.
Scope decisions 2–4 in the design page: the state menu, editing the title and description, and requesting reviewers.

Can start now: the pure intent and draft model, with mock tests for at-most-once dispatch and reconciliation.

Long-running goal prompt

Implement CR-7 (#326) in TypeSafeAI/modex. Read AGENTS.md, docs/status.md (Next #3, GitHub App), #319, this issue and docs/design/code-review.md, then verify main's SHA. Create your own worktree. Before any code that writes to GitHub, ask Val on this issue how writes are authorized (the gh login or a GitHub App) and which of decisions 2–4 are in scope. Stop on that question if it is unanswered. Meanwhile, build only the pure parts with red tests: draft and intent models keyed by account + repository id + PR + head SHA; at most one dispatch per live intent across double clicks and restarts; ambiguous outcomes reconciled by reading back, never retried blindly; stale heads, changed targets and outdated anchors rejected or re-confirmed. Once Val decides, add a separate main-owned write channel (never code-review:read) with permission and identity checks in main and no credentials in the renderer. Ship draft/submit first, then a merge that passes the reviewed head SHA and only offers supported methods, as separate reviewable increments. Unknown checks, model findings and unreadable policy never count as permission. Mock main/IPC and Electron tests should cover success, rejection, timeout, crash and reconciliation. A live write runs only against a disposable repository that Val authorizes. Open signed PRs; never merge a real PR from this work.

Follow-up / resume prompt

Resume CR-7. Confirm you own the worktree and fetch main. Re-read this issue for Val's answers on write authorization and decisions 2–4; don't assume them. Check whether #323/#324 have landed and whether their slots and anchors match your intent model. Re-run the at-most-once and reconciliation tests. Continue with one increment: drafts, submit, or the head-guarded merge. Post a checkpoint: SHA, results, decisions still open, and one next action.

Add the CR-8 acceptance matrix, Electron journeys and cross-slice reg…

Refreshed goal prompts — 2026-10-23 (`main` at `203a350`)

On main already:

capabilities() keeps comment, submit-review, request-reviewers, edit, state and merge unavailable, with this issue as the owner;
the code-review:read channel carries reads only.

Still open:

Nothing started. There is no branch, worktree or PR for CR-7.
Producer slices:
- feat(code-review): add PR overview, activity and evidence cards #393 (CR-4, in review) adds unavailable edit, reviewer and comment controls with explicit callback slots for this issue.
- CR-5's line anchors are in the unpushed cr5-remote-diffs worktree.
- Neither is on main.
Needs Val before any write code:
1. How writes are authorized. Reads go through the person's gh login. docs/status.md → Next Convention: one worktree per session under .worktrees/ #3 still says repository actions need a dedicated GitHub App, with an owner and a hosted service, which hasn't been decided. Writing through gh would need Val's explicit approval. There is no answer on this issue yet.
2. Scope decisions 2–4 in the design page: the state menu, editing the title and description, and requesting reviewers.
Can start now: the pure intent and draft model, with mock tests for at-most-once dispatch and reconciliation. Add the CR-8 acceptance matrix, Electron journeys and cross-slice regressions (#327) #372 (CR-8) already has a regression that the unavailable Review, Instructions, Merge and New tab actions dispatch nothing.

Long-running goal prompt

Implement CR-7 (#326) in TypeSafeAI/modex.

Read AGENTS.md, docs/status.md (Next #3, the GitHub App), #319, this issue and docs/design/code-review.md, then verify main's SHA. Create your own worktree with scripts/worktree.sh new.

Before any code that writes to GitHub, ask Val on this issue:
- how writes are authorized: the gh login or a GitHub App;
- which of decisions 2–4 are in scope.
Stop on those questions if they are unanswered.

Meanwhile, build only the pure parts, red tests first:
- draft and intent models keyed by account + repository id + PR + head SHA;
- at most one dispatch per live intent, across double clicks and restarts;
- ambiguous outcomes reconciled by reading back, never retried blindly;
- stale heads, changed targets and outdated anchors rejected or re-confirmed.

When #393 and CR-5 land, check that their callback slots and anchors match your model.

Once Val decides:
- Add a separate main-owned write channel. It is never code-review:read. Main checks permission and identity, and no credentials reach the renderer.
- Ship draft/submit first, then a merge that passes the reviewed head SHA and offers only supported methods. These are separate reviewable increments.
- Unknown checks, model findings and unreadable policy never count as permission.
- Cover success, rejection, timeout, crash and reconciliation with mock main/IPC and Electron tests.
- A live write runs only against a disposable repository that Val authorizes.

Run pnpm build && pnpm test && pnpm test:e2e. Update docs/status.md and docs/design/code-review.md. Open signed PRs, and never merge a real PR from this work without Val's go-ahead.

Follow-up / resume prompt

Resume CR-7.
1. Confirm you own the worktree and fetch main.
2. Re-read this issue for Val's answers on write authorization and decisions 2–4; don't assume them.
3. Check whether #393 and CR-5 have landed and whether their slots and anchors match your intent model.
4. Re-run the at-most-once and reconciliation tests.
5. Continue with one increment: drafts, submit, or the head-guarded merge.
Post a checkpoint: SHA, results, decisions still open, and one next action.

@BunsDev

@BunsDev: two decisions block CR-7's write code (`main` at `a10fcc4`)

No code that writes to GitHub will be written until these are answered here.

1. How are writes authorized?

(a) The person's gh login. Main would run fixed gh api mutation endpoints, as CR-2 does for reads. That needs your explicit approval, and an AGENTS.md entry like the LM-1 one.
(b) A dedicated GitHub App (docs/status.md → Next Convention: one worktree per session under .worktrees/ #3). That needs an owner and a hosted service destination first, so CR-7 writes would wait on it.

2. Which of decisions 2–4 (design page) are in #326's scope?

2: the state menu (C21: close, reopen, draft)
3: editing the title and description (C31, C35)
4: requesting reviewers (C38, C43)

Anything left out stays unavailable with its reason. Comments, submitting reviews and the head-guarded merge are in scope either way.

Where it stands

The draft/intent model is on main through Integrate Code Review and Markdown candidates with provenance fixes #404, unwired.
I checked it against the producer slices that have now landed:
- CR-5 (feat(code-review): add bounded revision-bound remote diffs (#324) #395): its LineAnchor adds commitSha, and the model rejected that shape. The fix makes LineAnchor one shared contract type and binds commitSha into the intent digest. A head-side anchor must name the revision's head. The PR for that follows.
- CR-4 (feat(code-review): add PR overview, activity and evidence cards #393): its comment, edit and requestReviewers slots fit the model. There's no slot yet for submitting a review, the state menu or Merge; the write increments add those.

Next action, once you answer: draft/submit through a separate main-owned write channel, then the merge as its own PR. A live write runs only against a disposable repository you name.

Coven-Harness: Claude Code / claude-opus-5-5

Decisions recorded (Val, 2026-10-25, answered in the CR-7 session)

Writes are authorized through the person's gh login (option a). Main runs fixed gh api mutation endpoints, like CR-2 does for reads. The first write PR adds the AGENTS.md entry recording this approval.
Decisions 2, 3 and 4 are all in scope: the state menu (close, reopen, draft), title and description edits, and reviewer requests.

Plan, as separate reviewable PRs:

A: a separate main-owned code-review:write channel, plus comment and review draft/submit.
B: the head-guarded merge, wiring the Add CR-7's unwired merge preflight #412 preflight to a fresh read.
C: the state menu, title/description edits and reviewer requests through the same channel.

All three are mock-tested only. A live write runs only against a disposable repository you name, and none of these PRs merge without your go-ahead.

Coven-Harness: Claude Code / claude-opus-5-5

@BunsDev

CR-7 increments are up for review; a live check needs a disposable repository

Following the 2026-10-25 decisions (gh login; decisions 2–4 in scope), the work is split into three stacked, signed PRs. None is merged.

CR-7: post comments and reviews through the gh login (#326) #413: comments and reviews on a separate code-review:write channel. Stacked on Bind CR-7 intents to CR-5's shared line anchors #410.
CR-7: merge the reviewed head through the gh login (#326) #414: Merge with sha = the reviewed head, offering only allowed methods. It carries Add CR-7's unwired merge preflight #412's preflight.
CR-7: state menu, edits and reviewer requests through the gh login (#326) #415: the state menu, title and description edits, and reviewer requests.

Each passed pnpm build && pnpm test && pnpm test:e2e locally. All testing so far uses a synthetic gh, so no real repository, PR, comment or review was touched.

@BunsDev, one question. Which disposable repository may the live check write to?

The check would open a throwaway PR there.
It would comment and review, close, reopen and draft it, edit its title, request a reviewer and squash-merge it.
The run would be recorded in docs/reviews/.

Nothing live runs until you name one. Please also say when any of #410 and #412–#415 may merge; none will be merged without your go-ahead.

Coven-Harness: Claude Code / claude-opus-5-5

CR-7 checkpoint, 2026-11-01 (`main` at `0a8c513`)

PR: #472. Docs and evidence only.

Re-run on main:

e2e/code-review-actions.spec.ts: 14 of 14 passed (part of a focused 51/51 run).
The CR-7 unit tests (code-review-writes, -write-ipc, -intents, -draft-buffer, -merge-preflight) passed inside the 223/224 Code Review unit run; the one skip is code-review-intents' Windows-only admission case. They use a synthetic gh.

Evidence docs corrected:

2026-10-25-cr7-write-submit.md and -merge.md no longer list merge and decisions 2–4 as future work.
2026-10-26-cr7-write-repair.md now records Integrate and repair CR-7 review writes and guarded actions #416's delivery:
- b3aad0d, 2026-10-27;
- the required check passed on head fffd07e (run 40415729228) and in the merge queue (run 40420085521);
- the post-merge push run on main (40423890925) failed its unit step and was then cancelled. That failure is not diagnosed here and is not counted as a pass.

Canary procedure: 2026-11-01-cr7-live-canary.md. It has not run.

It works on a throwaway PR in a disposable repository, through a pass-through gh that logs argv.
The steps: comment, review with a line comment, close, reopen, draft, ready, title edit, reviewer request, then squash-merge with the reviewed sha.
Each step records the preview, main's re-check, exactly one send and a read-back.
Closing checks: only the AGENTS.md endpoints, no DELETE, the login unchanged, and the token absent from everything Modex wrote.

Evidence by kind:

Synthetic gh: everything above.
Live write: none. Nothing ran against any real repository.

Waiting on Val: a disposable repository and a go, plus VoiceOver for the write dialogs.

Next action: run the canary step by step on that repository only.

Navigation Menu

Repository navigation

[CR-7][P1] Add explicit review actions and head-guarded GitHub merge #326

Outcome

Context / entry points

Items to do

Acceptance criteria

Validation / handoff

Long-running goal prompt

Follow-up / resume prompt

Independent review prompt

Refreshed goal prompts — 2026-10-22 (`main` at `0e674e7`)

Long-running goal prompt

Follow-up / resume prompt

Refreshed goal prompts — 2026-10-23 (`main` at `203a350`)

Long-running goal prompt

Follow-up / resume prompt

@BunsDev: two decisions block CR-7's write code (`main` at `a10fcc4`)

Decisions recorded (Val, 2026-10-25, answered in the CR-7 session)

CR-7 increments are up for review; a live check needs a disposable repository

CR-7 checkpoint, 2026-11-01 (`main` at `0a8c513`)

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[CR-7][P1] Add explicit review actions and head-guarded GitHub merge #326

Description

Outcome

Context / entry points

Items to do

Acceptance criteria

Validation / handoff

Long-running goal prompt

Follow-up / resume prompt

Independent review prompt

Activity

BunsDev commented on Oct 20, 2026

mariscate commented on Oct 22, 2026

Refreshed goal prompts — 2026-10-22 (main at 0e674e7)

Long-running goal prompt

Follow-up / resume prompt

BunsDev commented on Oct 23, 2026

Refreshed goal prompts — 2026-10-23 (main at 203a350)

Long-running goal prompt

Follow-up / resume prompt

BunsDev commented on Oct 24, 2026

@BunsDev: two decisions block CR-7's write code (main at a10fcc4)

BunsDev commented on Oct 25, 2026

Decisions recorded (Val, 2026-10-25, answered in the CR-7 session)

BunsDev commented on Oct 25, 2026

CR-7 increments are up for review; a live check needs a disposable repository

BunsDev commented on Nov 1, 2026

CR-7 checkpoint, 2026-11-01 (main at 0a8c513)

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Refreshed goal prompts — 2026-10-22 (`main` at `0e674e7`)

Refreshed goal prompts — 2026-10-23 (`main` at `203a350`)

@BunsDev: two decisions block CR-7's write code (`main` at `a10fcc4`)

CR-7 checkpoint, 2026-11-01 (`main` at `0a8c513`)