Skip to content

[Tracker][CR] Reconstruct the screenshot Code Review workspace end to end #319

Open

Description

@BunsDev

Outcome and scope

Build a functional Code Review workspace matching the four user-supplied desktop screenshots: PR inbox → overview/activity → split remote diff/file tree → explicitly requested AI review → human-confirmed GitHub action. Preserve Modex's existing desktop shell and local Changes workflow. This is an implementation backlog, not a claim that the feature has shipped.

Proposed implementation home: TypeSafeAI/modex, apps/desktop. The screenshots display both Modex and River Oaks PRs; the repository being reviewed is not necessarily the application owning the interface. Do not implement this in BunsDev/river-oaks, TypeSafe Computer, or the game iframe without an explicit scope change. #320 must record Val's confirmation of this placement before feature wiring. Read-only inspection and synthetic contract preparation can proceed before that decision.

Research limitation: the completed deep-research report for these four screenshots was not retrievable during this audit. Do not claim it was reviewed. This backlog is a fresh assessment of the supplied images, current source, existing issues and primary documentation. Reconcile a subsequently supplied report with current code; do not accept its proposals as implemented facts.

Verified baseline and reuse

Read-only audit baseline: main at cb33db5df00e235f68d36b5fac556dc30e0851f2. Re-fetch current main, issue discussions, open PRs and relevant scoped instructions before coding; this SHA is evidence, not a command to revert later work.

Reference states

These are original image pixel sizes, NOT established CSS viewport/DPR measurements. Originals remain user-supplied references, not repository assets. Do not commit unredacted screenshots, unrelated PR data, avatars or credentials without approval.

ID Original file Pixels Required state
R1 Screenshot 2026-10-18 at 15.38.42.png 1766×1057 Overview at top; title/body; stack/checks sidebar; floating PR composer
R2 Screenshot 2026-10-18 at 15.38.52.png 2048×1217 Scrolled activity; commits, comment composer, deployment preview; same selected PR
R3 Screenshot 2026-10-18 at 15.38.55.png 2047×1214 Review-instructions popover open above the activity view
R4 Screenshot 2026-10-18 at 15.40.30.png 2048×1215 Side-by-side PR description and Changes workspace; unified diffs and right file tree

Preserve composition, density, independent scrolling, hierarchy, selected/hover/focus states and existing Modex theme tokens. Use deterministic synthetic PR content with comparable line lengths. Screenshot-only controls with no verified backing capability must be explicitly unavailable, not fake-working. Native window chrome, voice input, proprietary cloud review APIs, stack mutation/rebasing, admin bypass and game integration are outside this initial scope.

Created issue graph and dependency rules

Slice Issue / deliverable Start prerequisite Completion / integration prerequisite
CR-1 #320 — reference/source contract and deterministic fixtures None; confirm proposed product placement before feature wiring Reviewed contract, reference access and placement decision
CR-2 #321 — authenticated main-owned PR reads #320 accepted contract Read/identity/race tests plus separately recorded live-read gate
CR-3 #322 — inbox, navigation and resizable workspace #320 accepted contract; can run alongside #321 #321 live adapter; navigation/layout evidence
CR-4 #323 — overview, activity, checks, stack and previews #321 and #322 accepted artifacts Safe rendering, provenance and R1/R2 evidence
CR-5 #324 — remote diffs, folding and file navigation #321 and #322 accepted artifacts Revision/anchor correctness, local Changes regression and R4 evidence
CR-6 #325 — native review, PR questions and instructions #321, #323 and #324; relevant accepted #273 contracts Runtime authority, lifecycle, coverage and R3/native evidence
CR-7 #326 — explicit GitHub writes and guarded merge #321, #323 and #324 Confirmation/receipt/race tests; separately authorized live-write gate
CR-8 #327 — integrated fidelity, accessibility and safety acceptance #320 for fixture/test design #321–#326 accepted implementations and their evidence

Parallel lanes: after #320, #321 and fixture-first #322 can proceed in separate owned worktrees. After their contracts are accepted, #323 and #324 can proceed in parallel. #325 and #326 are independent consumers: AI review does not grant or gate human GitHub write permission. #327 starts test design early but cannot claim integrated completion until its producer artifacts are ready.

Treat dependencies as accepted artifacts/behavior, not merely closed issue numbers. An accepted producer contract can unblock a consumer even while unrelated producer follow-ups remain open. Preparation can use agreed fixtures; live integration waits for the named producer. Parent tracking is not a blocking dependency. Do not add reverse dependencies from producers to #327, duplicate owners, cycles or blanket waits for all of #273 to close.

These relationships are explicit Markdown issue links and artifact gates. They are not claims of native GitHub blocking/sub-issue metadata or an automatically scheduled agent workflow. No agent assignment, merge or deployment was performed by creating this backlog.

Delivery gates

Gate A — read-only workspace: #320–#324, with #327's relevant tests. Users can navigate real PR data and inspect revision-bound diffs; AI/write controls stay capability-gated.

Gate B — native review: #325. Explicit user start, bounded context, runtime-enforced read-only authority, correct native lifecycle and visible coverage. Unsupported safety controls keep the feature unavailable.

Gate C — external writes: #326. Fresh identity/permission checks, explicit intent, expected-head merge protection and conservative unknown-outcome handling. No dependence on an AI verdict.

Gate D — full acceptance: #327. Reproducible visual/behavioral evidence plus named authenticated/native/write/human gates. These are delivery phases, not newly created GitHub milestone objects. Release/activation requires separate authorization.

Agent execution contract

  1. Read this tracker, the selected child, its dependencies and current AGENTS.md/scoped instructions. Establish repo, current branch/SHA, existing work, test availability and explicit owner. Use scripts/worktree.sh new <slug> and check; claim an existing worktree only through the documented ownership procedure. Never edit primary checkout or another agent's work.
  2. Select one unblocked, review-sized outcome. State the files/contracts to touch and the failing acceptance test. Implement the smallest change, run the targeted test, then relevant integration/regression checks. Do not rewrite already delivered dependencies. Parallel workers need disjoint ownership; shared bridge/types/shell changes require one integrator.
  3. Persist a short checkpoint at meaningful boundaries and before interruption: issue/PR, owner/worktree, branch and full SHA, accepted decisions, changed files, exact commands/results, evidence paths, unresolved risks, dependencies and one next action. Use the existing issue/PR or task-record mechanism; do not build a new orchestration framework. A summary is a navigation aid, never proof of current state.
  4. Stop on missing authorization, unknown worktree ownership, conflicting product decisions, absent reference assets needed for fidelity, unresolved security boundaries or repeated failures without a new diagnosis. After two unsuccessful attempts at the same failure, diagnose/escalate rather than repeating blindly. Respect execution/time/cost limits set by the caller; do not launch paid/provider runs without appropriate approval.
  5. Before completion, map every acceptance item to fresh evidence at the current SHA. Distinguish unit/mock, real Electron, authenticated GitHub, native-provider and human accessibility evidence. Never weaken tests or refresh goldens merely to turn a failure green. Preserve evidence outside directories Playwright wipes. Each child includes separate implementation, resume and independent-review prompts; the final review examines the actual diff/evidence, not the implementer's self-report.
  6. Run repository-required checks: pnpm build && pnpm test; UI changes also pnpm test:e2e. Use the pinned package manager. Document environmental blockers instead of claiming a pass. Update docs/status.md and a focused docs/reviews/ evidence record. Signed commits and a reviewable PR are the handoff; this backlog does not authorize merging, publishing a release, deploying, bypassing rules or executing foreign PR code.

Security and correctness gates

Main owns credentials, request validation, identity/repository scope, provider context and mutations. PR text, diffs, comments, filenames, branch names, deployment URLs and retrieved instructions are untrusted data. Rendering is inert; no remote HTML execution, credential leakage or remote-to-local path confusion. Read-only review must be enforced by the actual tool/process environment, not a sentence in a prompt. A GitHub check, AI verdict and human approval are separate facts. Unknown/stale/partial states never become success. Mutations require fresh authorization and explicit human intent; uncertain network outcomes require reconciliation, not automatic resend.

Primary sources and how they affect this plan

Long-running goal prompt

Deliver the Code Review reconstruction tracked by #319 in TypeSafeAI/modex, one unblocked child and one testable slice at a time. First read the latest tracker/child discussions, current main, AGENTS.md, source entry points and existing PRs. Start with #320: verify the proposed application placement and reference contract; do not turn River Oaks PR content into a game-client feature. Work only in an owned worktree. Reuse the shell, Changes presentation and native conversation contracts where they actually fit. Implement the selected child's acceptance criteria with failing-first tests, fresh evidence and small signed commits. Do not replace the stack, invent provider capabilities, run foreign PR code, expose credentials, or merge/deploy. Keep all untrusted review content outside executable authority. At every durable boundary record SHA, changed files, commands/results, remaining gates and one next action. Continue only within the caller's time/cost/authority limits; stop on a real blocker with a precise handoff. Finish with a reviewable PR and criterion-by-criterion evidence, not a claim based on a screenshot or an agent's own completion summary.

Follow-up / resume prompt

Resume #319 by reading its live issue graph, linked PRs and latest checkpoints. Re-establish repository, current main, worktree owner, branch and full SHA. Inspect the actual diff and rerun the relevant smoke test before trusting previous progress. Check whether prerequisite artifacts landed or changed; accept delivered contracts without needlessly waiting for unrelated issue closure. Pick the smallest remaining unblocked acceptance item and continue its child prompt. Preserve unrelated work, keep privacy/security gates intact, and do not replay external writes or provider runs whose outcomes are uncertain. If all automated criteria pass, identify remaining native, authenticated and human validation gates rather than declaring them covered. Update the checkpoint and status with observed evidence only. Do not merge or release.

Independent review prompt

Review the current #319 implementation against the live child issues and accepted R1–R4 contract. Read the actual source diff, dependency artifacts and fresh test evidence independently of the implementer's summary. Verify that existing Modex functionality was reused without remote-to-local authority leakage, and exercise account/head races, incomplete evidence, injected content and uncertain-action recovery. Inspect the UI and distinguish fixture, Electron, authenticated, native-provider, live-write and human accessibility proof. Report each acceptance criterion as verified, failed, deferred by an explicit decision or still unverified. Request focused fixes without expanding scope or authorizing merge/release.

Completion checklist

Activity

  1. BunsDev commented on Oct 20, 2026

    @BunsDev
    ContributorAuthor

    Gate A checkpoint 1 (2026-10-19): CR-1 ready for review

    The Gate A goal started from this tracker's prompt, scoped to #320–#324 plus #327's tests for those slices. The first slice, CR-1 (#320), is up for review as #334.

    What CR-1 delivers

    docs/design/code-review.md holds:

    • The contract (apps/desktop/src/shared/code-review.ts):
      • A locator, an account-scoped key, and a revision.
      • Read states: ready, partial, stale or unavailable.
      • Check rules: pending is never passing, and a newer run of a check replaces the older one.
      • Diff coverage.
      • Gate A capabilities: review and every write stay unavailable.
      • scopeOf and acceptsReply, so a slow reply for the wrong account, repository or head is dropped.
      • validateRequest, main's check on everything the renderer sends.
    • Synthetic fixtures: a made-up project; none of the screenshots' content.
    • A source map checked at file:line.
    • The reference manifest: checksums, scale, crop offsets, measured geometry, and typography and spacing.
    • An inventory of 82 controls, each with an owner and a test.
    • Seven open decisions for Val.

    Review

    1. First review (80ad189) requested changes:

      • The fixtures were the screenshots' PRs with the nouns swapped.
      • Three contract rules didn't hold as written.

      These were fixed in cf57a4b.

    2. Re-review: confirmed the fixes and left three small items, fixed in 6d77cbd.

    Evidence on 6d77cbd

    • Contract tests: 11/11.
    • Mutation checks: I broke the compiled contract 16 deliberate ways, and each break failed a test.
    • Full suite: pnpm build && pnpm test exited 0:
      • worktree scripts 109/109;
      • core 16/16;
      • desktop unit 1,672 passed, 16 skipped, 0 failed;
      • then 21, 10 and 16 in the remaining suites.
    • E2E: not run; there is no UI change.
    • CI on Define the Code Review read contract and fixtures (CR-1) #334: running.

    Still open

    For #321

    From GitHub's current REST docs (to confirm live):

    • Search: 30 requests a minute, at most 1,000 results per query.
    • PR files: the list stops at 3,000 files.
    • PR commits: the list stops at 250.
    • File contents: served raw up to 100 MB.
    • Search results: the schema lists a repository object with an id, but that hasn't been confirmed in a real response.

    Next action

    On Val's go-ahead:

    1. Land Define the Code Review read contract and fixtures (CR-1) #334 through the merge guard.
    2. Link the merged SHA on [CR-2][P1] Add a main-owned, revision-aware GitHub PR read service #321–[CR-8][P1] Verify Code Review fidelity, accessibility, recovery and end-to-end safety #327.
    3. Start CR-2 ([CR-2][P1] Add a main-owned, revision-aware GitHub PR read service #321) and CR-3 ([CR-3][P1] Build the PR inbox and resizable Code Review workspace #322) in separate worktrees.

    Nothing in those slices binds to the contract before it's on main.

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

  2. BunsDev commented on Oct 20, 2026

    @BunsDev
    ContributorAuthor

    Gate A checkpoint 2 (2026-10-20): CR-1 landed; CR-2 and CR-3 started

    CR-1 landed

    Decisions since checkpoint 1

    CR-2 and CR-3

    • The split: the design page's decision 4 fixes the interface. The renderer calls bridge.invoke("code-review:read", CodeReviewRequest), and main answers a CodeReviewReply, or null for cancel. That one BridgeCommands line is commit 372edbf, and both branches carry it.
    • CR-2 ([CR-2][P1] Add a main-owned, revision-aware GitHub PR read service #321): worktree cr2-read-service, at 372edbf on 43d5b9c. This session owns the main read service, the localOnly handler, and contract extensions.
    • CR-3 ([CR-3][P1] Build the PR inbox and resizable Code Review workspace #322): worktree cr3-inbox, on a separate branch, renderer only. E2E serves the fixtures through main's handler. CR-3 lands after CR-2 and wires the live read path then.

    Open for Val

    • Visibility before Gate A completes: should the Code Review destination appear in the app as soon as CR-3 lands, or stay hidden in packaged builds until [CR-8][P1] Verify Code Review fidelity, accessibility, recovery and end-to-end safety #327's acceptance, while staying on in dev and E2E? CR-3 builds it behind one switch that defaults to hidden in packaged builds. Recommended: hidden until Gate A completes.
    • The seven open decisions in the design page are still open.

    Next action

    CR-2: tests first for the read service (account switch, delayed replies, pagination and caps, revoked credentials, head changes, malformed paths), then the code-review:read handler.

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

  3. self-assigned this
    on Oct 20, 2026
  4. BunsDev commented on Oct 20, 2026

    @BunsDev
    ContributorAuthor

    Gate A checkpoint 3 (2026-10-20): CR-2 and CR-3 in review

    Open for Val

    Next action

    An independent review of #349, then landing it on Val's go-ahead. After that, merge main into cr3-inbox, wire the live read path, and run #322 AC6.

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

  5. mariscate commented on Oct 22, 2026

    @mariscate

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

    Where Gate A stands: CR-1 contract (#334, 43d5b9c), CR-2 read service (#366, plus cache denial #354/#361) and the CR-3 inbox/workspace host (#367, af49881) are on main; #320 and #321 are closed. apps/desktop/src/renderer/code-review/PrPane.tsx still shows placeholders for overview, diff and files, which are CR-4 (#323) and CR-5 (#324). Neither has started. #322 stays open for the R1–R4 comparison and human keyboard/VoiceOver review. Waiting on Val: the seven open decisions and whether the destination stays behind localStorage["modex.codeReview"] until Gate A completes. Five worktrees whose PRs already landed are still checked out (cr1-verify, cr2-cache-denial, cr2-ipc-acceptance, cr2-review, cr3-inbox-repair).

    Long-running goal prompt

    Coordinate the Code Review tracker #319 in TypeSafeAI/modex to Gate A completion, then Gates B/C. Read AGENTS.md, docs/status.md, docs/design/code-review.md and live #319–#327; verify main's SHA and open PRs. CR-1/2/3 are on main (43d5b9c, #366, af49881). Gate A's remaining work is CR-4 (#323 overview/activity/checks/stack) and CR-5 (#324 revision-bound remote diff and file tree). Run them in parallel, each in its own worktree from scripts/worktree.sh. Each binds to apps/desktop/src/shared/code-review.ts and the fixtures, and replaces a placeholder in renderer/code-review/PrPane.tsx. Have CR-8 (#327) build its acceptance matrix alongside them, not after. After both land, close #322 by running the R1–R4 comparison with populated content. Then start CR-6 (#325) and CR-7 (#326). Do not start CR-7 GitHub writes before Val decides how writes are authorized. Keep the seven design decisions and the visibility flag with Val; a control stays unavailable until its decision is made. One owner per shared file (contract, bridge, PrPane). Code PRs land only on Val's go-ahead through the merge guard, with signed commits. After each landing, post a checkpoint here: SHAs, PRs, evidence links, open decisions and one next action. Never merge, tag, release or flip the flag on your own.
    

    Follow-up / resume prompt

    Resume #319 coordination. Read the latest checkpoint here and on #322–#327, then git fetch and compare main with the last SHA recorded. List the open CR PRs and worktrees. For each, confirm the owner session, whether it is clean and whether the PR merged; ask owners to release/remove merged ones and never remove another session's worktree. Re-check which design decisions Val has answered and update the inventory owners in docs/design/code-review.md. Choose the one slice whose producer artifacts are accepted and unblock it. Record fixture, authenticated, live-provider and live-write evidence separately. Post a checkpoint with exact SHAs and the next action.
    
  6. BunsDev commented on Oct 23, 2026

    @BunsDev
    ContributorAuthor

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

    On main already:

    No Code Review code has landed since 0e674e7. The ten commits since then are CF work.

    Still open:

    cr1-verify, cr2-cache-denial, cr2-ipc-acceptance, cr2-review and cr3-inbox-repair are still checked out, although their PRs have landed. cr4-overview, cr5-remote-diffs and cr8-acceptance are active.

    Long-running goal prompt

    Coordinate the Code Review tracker #319 in TypeSafeAI/modex: finish Gate A, then run Gates B and C. Do not implement the child issues yourself.
    
    Read AGENTS.md, docs/status.md, docs/design/code-review.md and live #319–#327. Verify main's SHA and list the open PRs.
    
    What is on main: CR-1/2/3 (43d5b9c, #366, af49881).
    
    Gate A's remaining work is three pieces:
    - CR-4 (#323): in PR #393.
    - CR-5 (#324): unpushed commits in worktree cr5-remote-diffs.
    - CR-8 (#327): in PR #372.
    
    Get each one reviewed independently and landed on Val's go-ahead. Land #393 and CR-5 one after the other; the second merges main and keeps both slices' behaviour in PrPane. After each landing, have CR-8 re-run its journeys and update the matrix.
    
    Then close #322: run the R1–R4 comparison with populated content and the human keyboard/VoiceOver pass.
    
    Then start CR-6 (#325) and CR-7 (#326), each in its own worktree from scripts/worktree.sh. CR-6's Phase 1 isolation spike can start now. Do not start CR-7's GitHub writes before Val decides how writes are authorized.
    
    Ask Val where the R1–R4 references are now. Keep the seven design decisions and the visibility flag with Val; a control stays unavailable until its decision is made. Each shared file (contract, bridge, PrPane) has one owner.
    
    Code PRs land through the merge queue with signed commits, and only on Val's go-ahead. After each landing, post a checkpoint here: SHAs, PRs, evidence links, open decisions and one next action. Never merge, tag, release or flip the flag on your own.
    

    Follow-up / resume prompt

    Resume #319 coordination.
    1. Read the latest checkpoint here and on #322–#327. Run git fetch and compare main with the last SHA recorded (203a350).
    2. Check #393, #372 and the CR-5 branch: whether each merged, its CI state, and whether it has a review.
    3. List the CR worktrees. For each, confirm the owner session and whether it is clean. Ask owners to release or remove merged ones, and never remove another session's worktree.
    4. Re-check which design decisions Val has answered and where the references are. Update the inventory owners in docs/design/code-review.md.
    5. Pick the one slice whose producer artifacts are accepted, and unblock it.
    Record fixture, authenticated, live-provider and live-write evidence separately. Post a checkpoint with exact SHAs and the next action.
    
  7. BunsDev commented on Nov 1, 2026

    @BunsDev
    ContributorAuthor

    Code Review: one batched ask for Val (2026-11-01, main at 0a8c513)

    Every slice's code is on main. Lane C prepared everything that can be prepared without you, in #472 (docs and evidence only):

    Nothing live has run, and the app has made no GitHub write.

    Please answer here, or on the issue named:

    1. A repository and PR for CR-6's live review smoke ([CR-6][P1] Add native AI review, PR questions and scoped review instructions #325).
      • It uses your own claude login and reads through gh without writing.
      • Something small is best, to keep provider cost low.
      • Reply with owner/repo#n and "go".
    2. A disposable repository for CR-7's live-write canary ([CR-7][P1] Add explicit review actions and head-guarded GitHub merge #326).
      • It should be private and empty, with no required checks or merge queue, and allow squash.
      • You open the throwaway PR there yourself; the procedure lists the commands.
      • The canary posts a comment and a review, closes, reopens, drafts and readies the PR, edits its title, requests a reviewer if a second account is available, and squash-merges it.
      • It never runs on TypeSafeAI/modex. Reply with owner/repo and "go".
    3. Decision 1 (avatars, C16/C81): login only (D1), or fetch avatars?
    4. Decision 5 (Viewed toggle, C65): local only, or GitHub's viewed state?
    5. Decision 6 (new-tab button, C07): what does it open on this surface? Until you decide, it stays unavailable.
    6. Decision 7 (access-mode indicator, C82): the reference's PR composer shows full access, but review is read-only. Should this composer show an access mode at all, and if so, which? Today the panel states the confinement in words and draws no indicator.

    Also yours, whenever you're ready:

    Agent follow-ups that don't need you:

    • C22 still uses the row's locator (D6);
    • the a11y spec covers only CR-3's controls (C76);
    • seven built controls have no asserting spec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions