Skip to content

Add main's Code Review read service through gh (CR-2) - #349

Closed
BunsDev wants to merge 8 commits into
mainfrom
cr2-read-service
Closed

BunsDev wants to merge 8 commits into
mainfrom
cr2-read-service

Conversation

@BunsDev

@BunsDev BunsDev commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Superseded by #366, which carries all reviewed CR2 work and the merged #354/#361 repairs onto current main while preserving the newly landed CF7 changes. The original evidence below remains historical; #366 records fresh integration validation. #321 remains open until the replacement lands and acceptance is audited.

Current integration status

The visibility-loss repair #354 and production Electron IPC acceptance test #361 are merged here, now at 8882aadbe70d54146f6edcc469d754849f43cfb8. Denial/not-found/identity failures invalidate retained PR data and concurrent sibling reads while preserving unrelated repositories and ordinary offline/rate-limit fallback.

#361 passed the required macOS unit/E2E check on c180c72187e9dbbeaa3df5815161774f3babb358 before merging. Its actual production preload/IPC/service test covers inbox → overview → files, distinct repository identities, foreign-account rejection and an unchanged checkout. Removing the production registration made it fail. Local build/typecheck and 1,955 unit checks passed (16 skips); the restored focused Electron test passed. Validate this resulting parent head and its main merge group before closing #321.

The original implementation and its SHA-specific evidence follow; their counts are not results for the current head.

CR-2 of tracker #319, for #321: main's read service on the code-review:read channel. It serves the contract #334 defined, and CR-3 (#346) binds its UI to it. Read-only. No checkout, no comment or review, no merge, no model call. Nothing is visible in the app until CR-3 lands.

How it reads

  • Whose login: Val's own gh login ([CR-1][P1] Establish the Code Review reference contract and deterministic fixtures #320 decision 3). Modex adds no sign-in, and never sees, stores or asks for a token.

  • The command: each read is gh api <fixed path> --hostname github.com --method GET --include, run through execFile with no shell.

  • What gh.ts refuses:

    • any path that isn't user, the issue search, or under repos/;
    • any flag, host, space, {owner} placeholder or dot segment;
    • GH_DEBUG, which is never forwarded.

    Nothing gh prints to stderr is kept.

  • The channel: handle("code-review:read", …, { localOnly: true }) in index.ts, so paired desktops can't reach it. The payload is untrusted until validateRequest has rebuilt it. Browser development and the Store demo answer the channel without GitHub.

The design page's new Read service section holds the details: principal and scopes, how each gh failure maps to a reason, keys and generations, revisions, limits and memory, and refresh.

What's in it

Commit What
372edbf The BridgeCommands line, "code-review:read": { req: CodeReviewRequest; res: CodeReviewReply | null }. cancel answers null.
a34ef72 Contract changes for what GitHub actually returns (below).
ae6ea30 The Store demo answers the channel as unavailable. Its handler map has to cover every channel.
197276e, f2c2de7, d31f661 The service, then the fixes from the review and the re-review: src/main/engine/code-review/{gh,github,read-service}.ts, the handlers, three test files, the design page and status.md.

CR-3 (#346) carries the first three commits as cherry-picks.

Contract changes:

  • stackHint becomes boolean | null, because search results name no base branch.
  • Unavailable gains head-changed.
  • parseRevision is exported.

Acceptance, criterion by criterion

The fixture evidence comes from a synthetic GitHub: two accounts, and two repositories that both have a #73. The authenticated evidence is separate, under AC5.

  • AC1: list → detail → files through main's handler, with no local mutation.

    • code-review-ipc.test.ts slices handle("code-review:read", …) out of compiled index.js and checks it is registered localOnly. It drives that handler closure, the real service and the real gh transport against a fake gh executable: connection, inbox, overview of both #73s, then files.
    • The test does not go through Electron's ipcMain. The shared handle() wrapper (trusted sender and document epoch) is covered by the existing main tests.
    • Every endpoint called is on the allowlist, and nothing is written except the fake's own log.
    • A scan of the read path's sources finds no fs or git.ts import, no write call, no -X, graphql or write method, and no Discard.
  • AC2: identical numbers and account switches.

    • Two #73s whose answers arrive in the opposite order to the requests each answer only their own request, and acceptsReply rejects each for the other.
    • Main fingerprints gh's hosts.yml, which gh auth switch, login and logout rewrite.
      • It rechecks the account when the fingerprint changes, and confirms the account after every read before replying or keeping anything.
      • A read whose fingerprint moved while it ran is refused, even if gh is back on the same account.
      • Where the file can't be read, it confirms with a conditional gh api user (a free 304). A check that fails, fails the read.
      • A switch that leaves the file alone is caught when the 30 s TTL runs out.
    • A switch during a slow read, with no connection read in between, answers signed-out with nothing in it. This holds whether the read was aborted or gh finished it as the new account.
    • A key for the old account is refused without making a request. The read that noticed the switch answers under the new account.
    • After a switch, everything learned under the old account is gone, including its rate-limit state.
  • AC3: recoverable states, with no secret leakage.

    Case Result
    401 (revoked) signed-out, and the identity is dropped
    404 not-found
    403 denied
    403 with SSO denied, with a message to authorize gh for single sign-on
    Malformed or non-github.com link invalid, with no request made
    Timeout timeout
    Primary rate limit rate-limited with retryAfter; main makes no request until the reset
    Secondary limit (429 with Retry-After) rate-limited
    Search past GitHub's own time limit partial, reason timeout
    Search past 1,000 results capped; p35 and foreign cursors are refused
    Files past 3,000 capped
    Patch missing with no changed lines binary (covered)
    Patch missing with changed lines too-large (missing)
    Offline refresh stale, with the last good answer

    Fake gh stderr and bodies carrying ghp_… strings never reach a reply.

  • AC4: head changes.

    • If the head moved before a files page, the page is stale with the last good copy, or unavailable: head-changed when there is none.
    • If it moved during the page, the page is stale: head-changed.
    • Checks for an old head are stale, so summarizeChecks calls them incomplete.
    • context refuses a revision the PR never had, without making a request.
    • A name that now belongs to a different repository answers not-found.
  • AC5: authenticated read-only smoke test. Recorded separately below.

  • AC6: no regressions, no broad permissions, no write endpoint, no cloud service.

    • The full suite passes; numbers are under Evidence.
    • The only new process is gh api … --method GET.

AC5: authenticated smoke test (TypeSafeAI/modex, authorized by Val on 2026-10-20)

apps/desktop/.probes/cr2-live-smoke.mjs is git-ignored. It runs the compiled service with the real transport through Val's gh login and prints no token, header value or response body.

Account BunsDev (id 68980965) on github.com, gh 2.98.0, API version 2022-11-28
Repository TypeSafeAI/modex, stable id 1386876170
PR #334 head 5106c0d8e56d4ea3f72d37c1b1afe45f9d30a57e, base main at e2d7703a63e76d08373fb165b50c010510b56017, merged; 2026-10-20T19:59:15Z
PR #330 head 6f3a6c98c7cdaaacee25570c12a0c303ff4c8b0d, which includes a real bot review

Results, the same on both PRs:

The first live run found two mapping gaps, both fixed here with failing tests first:

  • Cross-references have no id of their own.
  • The timeline spells review states in lower case.

After the fix, real timelines for #295, #316, #330 and #334 map with nothing dropped.

Also checked live, and written into the design page:

  • Search results carry repository_url but no repository object, though the schema lists one.
  • A 304 exits 1 but still prints its status line, and isn't charged.
  • Under the raw media type, a directory comes back as JSON.

Evidence

  • Red first:

    • Each new test file failed to compile before its module existed (Cannot find module …/gh.js, then …/read-service.js).
    • The cross-reference case and the lowercase-review case each failed before their fixes.
  • Code Review tests: 48 pass across the four files: the contract test plus the three new ones.

  • Mutation checks: I broke the compiled read path and the contract 44 ways, one at a time, and every break failed a test. That includes 19 aimed at the review and re-review fixes. Among them:

    • an old-account reply let through;
    • caches kept across a switch;
    • no head check before or after a files page;
    • checks on an old head counted as current;
    • rate limits not held;
    • the search cap not reported;
    • context served for any revision, or a directory served as a file;
    • a reborn repository accepted;
    • identity never rechecked;
    • no concurrency bound;
    • no conditional requests;
    • a 401 that keeps the identity;
    • SSO read as plain denied;
    • duplicate request ids;
    • the stack hint never learned;
    • no stale fallback, or a stale answer for a cancelled read;
    • binary and too-large swapped;
    • pending reviews listed;
    • GH_DEBUG forwarded;
    • another host allowed;
    • offline read as failed.
  • Full suite: pnpm build && pnpm test exited 0 on d31f661 (base 43d5b9c, macOS, Node 24.18.1):

    • worktree scripts: 111/111;
    • core: 16/16;
    • desktop unit: 1,753 passed, 16 skipped, 0 failed;
    • then 21, 10 and 16 in the remaining suites.

    A trial merge with current main (ac7f743) is clean, and the merge queue tests the combination.

  • E2E: not run locally. There is no renderer change, and free disk was low (2.4 to 7.5 GB). CI's unit + e2e (macOS) runs it.

Review

An independent review of 197276e requested changes: one AC2 blocker, five should-fix and a dozen nits. All of them are fixed in f2c2de7, each with a failing test first.

A re-review of f2c2de7 found two more AC2 holes in the new after-read check, plus a partial fix for item 5. They are fixed in d31f661. Both replies are in the PR comments.

Not done here

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

The design page's decision 4: one channel, CodeReviewRequest in and
CodeReviewReply out. A cancel request answers null. No handler is
registered yet; CR-2's read service adds it, and CR-3 binds to this line.

Coven-Harness: Claude Code / claude-opus-5-5
- PrSummary.stackHint is boolean | null. A search result names no base
  branch (checked live: no repository object either), so main knows the
  hint only for PRs it has read itself.
- Unavailable gains head-changed: a files or checks read whose PR moved
  past the asked revision, with no earlier read kept to show as stale.
- parseRevision is exported, so main checks GitHub's base ref and SHAs
  with the same rule the renderer's requests go through.

Coven-Harness: Claude Code / claude-opus-5-5
The Store demo's handler map covers every bridge channel, so the new
channel broke its build. The offline demo has no GitHub and answers it
as unavailable, like its other account and system channels.

Coven-Harness: Claude Code / claude-opus-5-5
code-review:read is registered localOnly. Every payload passes
validateRequest before anything runs. Reads go through the person's
own gh login: gh api <fixed path> --hostname github.com --method GET
--include, with no shell, no new sign-in and no token in Modex.

- gh.ts runs gh and reads its status line, headers and body. It refuses
  any path that is not user, the issue search or under repos/, and
  keeps nothing gh prints to stderr.
- github.ts maps GitHub's REST answers to the contract, checking and
  bounding every field.
- read-service.ts keys each read by the gh account (rechecked after
  30 s, conditionally) and the repository's stable id. It learns names
  only from its own reads. An account switch aborts reads in flight and
  drops everything learned. A files page is bracketed by PR reads, so a
  moved head reads as stale head-changed, and checks for an old head
  are stale. It has conditional caching, bounded concurrency,
  rate-limit backoff, cancellation, and last-good answers kept as
  stale. It never polls.
- Browser development answers the channel with a disabled service.

Checked live on TypeSafeAI/modex with Val's authorization, and written
into the design page:
- Search results carry no repository object.
- A cross-reference has no id of its own.
- The timeline spells review states in lower case.
- The raw media type answers a directory as JSON.

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

vercel Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
modex Ready Ready Preview Oct 21, 2026 2:47am UTC

Request Review

The independent review of 197276e requested changes.

- AC2 (blocker): a gh auth switch inside the 30 s identity window went
  unnoticed, so reads ran as the new account but were stamped as the
  old one. Main now fingerprints gh's hosts.yml, which switch, login
  and logout rewrite. It rechecks the account when that changes, and
  confirms the account after every read before replying. Where the
  file can't be read, it confirms with a conditional gh api user (a
  free 304). The read that notices a switch goes on under the new
  account, and a switch clears rate-limit state.
- AC3: when the account can't be checked (offline, slow, limited), the
  last confirmed account still scopes kept answers, which come back
  stale. Repository lookups all settle before one failure is raised. A
  2xx that gh exits non-zero after is failed, not a short file. A short
  files page is partial, and search pages after the first never claim
  complete.
- AC4: checks are bracketed by PR reads, like files.
- Nits: hasOwn enum lookups, strict ISO timestamps, a 429 without
  Retry-After waits 60 s, a recreated repository is re-resolved,
  duplicate request ids are rejected, a cancelled read never starts gh,
  and a forced identity check never joins an older one.
- AC5: the smoke probe and its results are recorded in
  docs/reviews/2026-10-20-cr2-read-smoke.md.

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

BunsDev commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Independent review of 197276e: findings and fixes

An independent reviewer requested changes. It worked read-only against fake transports, and its probes were scratch scripts outside the repository. I confirmed each finding, wrote a failing test first, then fixed it. The changes are in f2c2de7.

# Finding Severity / AC Fix
1 A gh auth switch inside the 30 s identity window went unnoticed. Reads ran as the new account but were stamped as the old one. Blocker, AC2 Main fingerprints gh's hosts.yml, which switch, login and logout rewrite. It rechecks the account when the fingerprint changes, and confirms the account after every read before replying. Where the file can't be read, it confirms with a conditional gh api user (a free 304). New tests cover a switch with no connection read in between, and the no-hosts-file fallback.
2 Once the identity TTL had passed, an offline, slow or rate-limited refresh dropped the kept answer. The inbox replied with a null account. AC3 When the identity check itself fails transiently, main answers from the last confirmed account: kept answers come back stale under that account's scope.
3 group() left rejected repository lookups unhandled. AC3 Promise.allSettled, then the first real failure is raised. The test listens for unhandledRejection and gets none.
4 A 2xx that gh printed and then exited non-zero was served as a complete file. AC3 That case is now failed. A 304, and a 4xx gh printed in full, are unaffected.
5 Coverage said complete when rows were missing. AC3 A files page that stops short of the PR's count is partial. Search pages after the first never claim complete; the renderer dedupes by key against total.
6 Checks weren't re-read against the head afterwards. AC4 Checks are now bracketed by PR reads, like files.

Nits, all fixed with tests:

  • Enum lookups use Object.hasOwn, so constructor and __proto__ map to unknown, and the reply can cross IPC.
  • Timestamps must be ISO 8601.
  • A 429 without Retry-After holds requests for 60 s.
  • A change of login, or a switch of account, clears rate-limit state.
  • A recreated repository is re-resolved.
  • The read that notices a switch is no longer aborted.
  • Duplicate request ids are rejected.
  • activity's loaded counts the provider's own items.
  • A read cancelled while waiting for a slot never starts gh.
  • A forced identity check never joins one that started before it.
  • Malformed search rows read as failed, not denied.
  • A check list that stops short of its total, without hitting main's page limit, is failed, not capped.

Also from the review:

  • AC1 wording: the test drives main's compiled handler closure and checks it is registered localOnly. The shared sender check in handle() is covered by the existing main tests. The PR body now says that.
  • AC5 reproducibility: the smoke probe and its results are committed as docs/reviews/2026-10-20-cr2-read-smoke.md.

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

const revision = overview.result.value?.revision;
evidence.revision = revision;
evidence.overview = overview.result.value && { state: overview.result.value.state, totals: overview.result.value.totals, headRepo: overview.result.value.headRepo };
const again = step("overview again (conditional)", await read({ op: "overview", key }));
The re-review of f2c2de7 found two ways the after-read check still let
another account's rows through:

- A: a switch away and back within one read. gh answered the original
  account again, so the read was accepted. Each read now records the
  hosts.yml fingerprint it began under. If it moved during the read,
  the answer is refused and nothing is kept, even if the account
  matches now.
- B: a confirming check that failed passed the read. It now fails it.

Also:
- An inbox's groups are kept only after the check.
- A login seen to change stops the offline fallback to the last
  account's answers.
- The fingerprint includes the file's change time.
- The contract gains mergePages, which joins an inbox group's pages,
  deduplicated by key, and calls the list complete only when the
  distinct rows reach the provider's total.

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

BunsDev commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Re-review of f2c2de7: findings and fixes

The re-review confirmed the six original findings fixed (5 only partly) and found two new AC2 holes in the after-read check. Each fix below started with a failing test; the changes are in d31f661.

# Finding Fix
A A switch away and back within one read was accepted: gh answered the original account again. Each read records the hosts.yml fingerprint it began under. If the fingerprint moved during the read, the answer is refused (failed, "sign-in changed") and nothing is kept, even when the account matches now.
B A confirming check that failed passed the read. A failed check now fails the read, with the check's own reason. Without a hosts file to watch, a failing read under an unnoticed switch can't fall back to the old account's kept answer either.
— The inbox kept its groups before the check. Groups are kept only after the check.
C After a seen login change, an offline read showed the last account's kept answers. No fallback once the fingerprint shows the login changed.
5 (partial) Nothing in CR-2 deduplicated inbox pages. The contract's mergePages joins a group's pages, deduplicated by key. The list is complete only when every page was read in full and the distinct rows reach total. The test reproduces the review's case: #129 repeated, #130 skipped.
nit Same-size rewrite within one timestamp tick. The fingerprint includes the file's change time.

Two things are left as they are, and the design page records both:

  • The blocking statSync: it reads one small local file. I kept it synchronous.
  • The limit without a hosts file: a switch away and back within one read can't be seen.

Mutation checks: 44, six of them aimed at these fixes. Every break fails a test.

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

const release = github.hold(/^search\//);
const inbox = read({ op: "inbox", query: "" });
await until(() => github.inFlight >= 2);
github.user = KIT; // the search runs as Kit
@BunsDev

BunsDev commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Re-review of d31f661: approved

The independent reviewer approved this commit. It confirmed AC2, AC3 and AC4 against the earlier findings and some new probes, run with and without a readable hosts.yml.

Fixed, and confirmed:

  • A: a switch away and back within one read. Fixed wherever hosts.yml can be read, or doesn't exist. An unreadable file is the documented exception.
  • B: a failed confirming check. Fixed in both modes.
  • Kept answers: written only after confirm().
  • C: a switch followed by offline.

Open, none blocking:

  1. mergePages: a PR closing between pages can balance a skipped row, so the list reads complete. The fix is to also require every page to report the same total. Nothing calls it yet; CR-3 fixes this before relying on it (noted on [CR-3][P1] Build the PR inbox and resizable Code Review workspace #322).
  2. Only when hosts.yml can't be read: offline within the 30 s window answers unavailable, but after it stale.
  3. Hardening, unconfirmed:
    • keep() doesn't recheck the generation after confirm().
    • Kept inbox and resolve answers aren't keyed by account; clearing them on a switch is what keeps accounts apart.
  4. By design, documented in the design page:
    • the synchronous statSync;
    • an unreadable hosts.yml can't catch a switch away and back.

The reviewer also advised that the renderer retry a "sign-in changed" refusal at most once.

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

@github-code-quality

github-code-quality Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Code coverage overview

Languages: TypeScript

TypeScript / code-coverage-agent

main 4e9d965 cr2-read-service 2ba1462 +/-
64% 65% +1%
View coverage of the 7 most impacted files.
File main 4e9d965 cr2-read-service 2ba1462 +/-
apps/store-desk...rc/main/demo.ts 95% 95% 0%
apps/desktop/sr...shared/types.ts 100% 100% 0%
apps/desktop/sr...ain/commands.ts 86% 87% +1%
apps/desktop/sr.../code-review.ts 99% 100% +1%
apps/desktop/sr...read-service.ts 0% 98% +98%
apps/desktop/sr...eview/github.ts 0% 99% +99%
apps/desktop/sr...de-review/gh.ts 0% 100% +100%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.


Updated October 21, 2026 01:28 UTC

@BunsDev
BunsDev added this pull request to the merge queue Oct 20, 2026
@BunsDev
BunsDev removed this pull request from the merge queue due to a manual request Oct 20, 2026
* Invalidate retained PR data after authoritative visibility loss

* Invalidate shared read resources without losing partial inbox results
* Keep keyboard focus when an approval is answered, its turn folds, Stop ends it, or Jump to latest is used (CF-8) (#342)

* Keep keyboard focus when an approval is answered and its turn folds

The CF-8 keyboard walkthrough found that pressing Approve or Deny with
the keyboard dropped focus onto the page body: the answer buttons
unmount once pressed, and a finished turn then hides its folded work.
A keyboard or VoiceOver user lost their place twice.

Focus now moves to the card's answer line ("Sending decision…", then
"Approved" or "Denied"). When the finished turn folds work that holds
focus, it moves to the turn's "Worked for" toggle. Focus elsewhere,
such as the composer, is left alone.

Red first: both keyboard tests failed on main (answer line inactive).
e2e/keyboard-answers.spec.ts passes 3/3; the full desktop e2e passes
366 with 6 opt-in skips.

Part of #281 (CF-8), tracker #273.

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

* Hand focus to the message box when a keyboard Stop ends the turn

The second walkthrough probe found the same defect at Stop: the button
goes away when the turn ends, and focus fell onto the page body. When
Stop was pressed with focus on it, focus now moves to the message box,
where the next message is typed.

Red first: the new keyboard test failed on the previous commit
(composer inactive); keyboard-answers.spec.ts passes 4/4 and the full
desktop e2e passes 367 with 6 opt-in skips.

Part of #281 (CF-8), tracker #273.

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

* Hand focus to the message box after Jump to latest from the keyboard

The third walkthrough probe found that Jump to latest, once activated,
goes away at the bottom and left focus on the page body. When the
button held focus, focus now moves to the message box there.

Red first: the new test failed (message box inactive). The keyboard
spec passes 5/5, app.spec with it 19/19, and conversation-continuity
plus typed-turns, which also scroll, 8/8.

Part of #281 (CF-8), tracker #273.

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

* Restore main's status.md after the #339 merge, keeping only this PR's entry

The merge kept both sides of a block #339 had moved, which duplicated
the released entries. The page is now main's, plus this PR's one entry.

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

* Show a Codex web search's query and the results Codex reported (CF-5) (#344)

A live codex-cli 0.163.0 capture (test/fixtures/codex-websearch-0.163.0.json)
shows the webSearch item starting with an empty query, no action and
no results, and completing with the query, a search action and
text_result entries (title, url, domain, snippet). Modex titled the row
"search " from the empty start, read neither action nor results, and
stored the JSON string "" as its output.

codexWebSearch (shared/evidence.ts) keeps the query, the action and
results with an http(s) link, each text bounded, and counts what it
drops. The Codex backend titles the row "search the web" until the
query arrives, then "search <query>", and carries the search on the
tool item. Expanded, the results show as links with snippets, marked
data-provenance="codex-search" and labelled as unchecked. The renderer
checks each link again before showing it, and nothing is fetched until
a link is clicked. Older searches no longer show "".

Red first: the three evidence tests and the Codex backend test failed
on a stub, and the e2e test failed ("done", not "2 results"). Now
evidence, codex-backend and conversation-events pass 69/69, the
conversation-evidence spec passes 7/7, and the desktop unit suite
1,709 passed with 16 skipped.

Part of #278 (CF-5), tracker #273.

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

* Land main through a merge queue (#345)

main requires branches to be up to date, so every landing sent each open
PR back through CI. On 2026-10-19 that cost #309 three extra macOS runs,
and on 2026-10-20 #336, #340 and #343 one or more each.

The ruleset file gains a merge_queue rule (squash, all-green groups, up
to 3 groups building and 5 PRs per merge, a 90-minute check timeout,
since the macOS job alone may take 45). It drops the up-to-date
requirement, which the queue replaces: a queued PR is tested on top of
main and the PRs ahead of it.

CI runs on merge_group, so each queued group reports the required
unit + e2e (macOS) check; without it the queue drops every PR. The
iPhone job and the coverage upload skip queued groups: neither is
required, the PR already ran them, and main's push run repeats them.

README, AGENTS.md and status.md say PRs land through the queue. The live
ruleset (23989202) takes this file once this lands, so the queue starts
only after main can report checks for it.

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

* Say the phone no longer shows a withdrawn approval as declined (#343)

human-requests.md still said the iPhone shows a withdrawn approval as
declined. Since #335, mobileItem (companion.ts) sends a withdrawn,
sending or uncertain approval to the phone as a notice that says so.
The companion fallback test now covers the withdrawn case, which had
no test (CF-6 #279, finding 7).

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

* Note that the merge queue needs auto-merge, and list the keyboard Jump to latest CI failure (steward cycle 17) (#347)

The first PR queued after #345 failed to enqueue: "Auto merge is not
allowed for this repository". gh pr merge adds a PR to a merge queue by
enabling auto-merge, so the repository now allows it, as OpenCoven/coven
does, and README says why.

e2e/keyboard-answers.spec.ts:88 (#342) failed in three CI runs of #345 in
a row and passed on #342, on main and 25/25 locally. It goes under Known
rough edges with what the failure snapshot shows and a link to the
analysis on #342.

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

* Prove a queue-landed branch merged when its tip merges cleanly into the landed tree (#350)

Since #345, main lands through a merge queue, which squashes each PR onto main as it stands at
the PR's turn. A branch that merged main into itself, then landed behind another PR, had no proof
that worktree.sh remove would accept:
- the squash tree also holds the PRs ahead of it, so it is not the tip's tree;
- the branch's merge commits rule out the binary-diff proof.

cf6-docs-followup (#343) hit this on 2026-10-20 and had to be pointed at its squash commit by hand.

The new merge-tree proof merges the exact PR tip onto the squash commit's parent with Git's own
merge. It accepts only a clean merge (exit 0) whose tree is exactly the landed tree. A configured
merge.<name>.driver turns the check off, because a driver can drop a side's changes and still
report a clean merge.

New tests:
- a queued branch with merge commits is proven and removed, and its archive ref holds the tip;
- a wrong head, a wrong base, a different tree, a conflicted merge (even when the squash holds
  exactly the conflicted tree) and a configured driver that keeps main's side each stay
  not-proven-merged.

Checked against #343's real branch and PR record in a throwaway clone. The old proof returns null.
The new proof returns merge-tree with tree 6fc493b, the tree of 0a67354.

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

* Hold Jump to latest when the composer grows or a Page Up is still moving (#352)

* Hold Jump to latest when the composer grows or a Page Up is still moving

e2e/keyboard-answers.spec.ts:88 (#342) failed on slow CI runners: after
Enter, Jump to latest stayed. A trace in the app found two causes.

- A scroll event is delivered before the browser reports a resize. If
  the composer grew right after the jump (a follow-up chip), onScroll
  measured a shorter view and stopped following. It now keeps following
  while the view has not moved above where auto-scroll put it, as
  follow() already did.
- A Page Up still animating keeps applying its remaining distance after
  the page sets a new position: plain assignment, scrollTo instant and
  an overflow toggle all lose to it. For a second after a jump, a scroll
  with no wheel, key, pointer or touch input since the jump is that
  leftover motion, and the view pins back to the bottom. The reader's
  own scroll still wins at once.

Red first, each test alone on main's ThreadView: the composer-growth
test failed (the button stayed), the Page Up test failed (the view went
back up 138 px), and the scroll-up guard passed. With the fix,
keyboard-answers passes 24/24 (8 tests x 3), and app, conversation-
continuity, typed-turns, conversation-rendering and layout 43/43.

Part of #281 (CF-8), tracker #273.

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

* Move the Jump to latest rough edge to the resolved list, with its cause

The entry #347 added leaves Known rough edges, as written, under #352,
with the two causes the trace found.

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

* Invalidate retained PR data after authoritative visibility loss

* Say whether a check verified a turn's last change (CF-7 verification) (#351)

* Say whether a check verified a turn's last change, in its task record (CF-7)

verification() in shared/task-context.ts reads a turn's tools in
transcript order:
- A check is a known test, build, typecheck or lint command, read
  inside sh/bash/zsh -c with quotes respected. An exit-zero echo or
  the model saying "tests pass" is not one.
- A check passes only as succeeded with exit code 0 or none reported.
- A check goes stale when an edit that was not denied, or a command
  that might write, runs after it.
- The turn reads verified, failed, unknown, stale or none.

taskRecord carries it, and the note gets a Verification section that
marks stale checks. Anything short of verified goes under Not settled;
a failure a later fix superseded does not. Plan steps keep the CLI's
statuses: neither CLI links a check to a step.

Red first: five of six new tests failed on a stub, then edits and
displayCommand on theirs. task-context passes 13/13; the desktop
unit suite 1,723 passed, 16 skipped.

Part of #280 (CF-7), tracker #273.

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

* Show a turn's verification beside Save to Notes (CF-7)

A finished turn's footer reads, for example, "Verified: pnpm test passed
after the last change" or "Not verified: pnpm test ran before a later
change". It also covers a check that failed, a check that did not say
how it ended, and a turn whose changes no check followed. A check
shows as the script its shell ran, with the exact command on hover. A
turn that changed nothing and ran no check shows nothing.

Red first: the new e2e test found no verification lines (expected 2,
received 0). task-to-notes.spec.ts passes 3/3.

docs/task-context.md gains a Verification section, and status.md an
entry.

Part of #280 (CF-7), tracker #273.

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

* List #352's Jump to latest fix under On main, unreleased (steward cycle 19) (#353)

#352 changed what readers see: after a jump, the transcript no longer stops short of the bottom
when the message box grows or a Page Up is still scrolling. It moved the CI rough edge to the
resolved list but added no unreleased entry, so the next release review would not have listed it.
The entry sits beside CF-8's keyboard entry and links the resolved list's account of the causes.

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

* Mark the CF-7 verification plan done in #351 (steward cycle 20) (#356)

#351 landed every task in docs/superpowers/plans/2026-10-20-cf7-verification.md, but the plan
still read as not started. The five boxes are ticked from #351's own evidence:
- the red and green unit tests;
- the footer e2e (3/3);
- task-context.md and status.md updated;
- pnpm build and typecheck passed locally;
- the full e2e suite passed in CI, on the PR and in the merge queue (run 39508701952). The local
  run stopped when the disk filled.

A dated status line says that linking a check to a plan step stays open on #280.

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

* Invalidate shared read resources without losing partial inbox results

This branch was successfully deployed

1 active deployment
Preview — 8882aadb Deployed Oct 21, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant