Skip to content

feat(fork): port GitHub Issues onto v2 - #179

Merged
lukemaj merged 1 commit into
fork/v2from
feat/171-issues-v2
Oct 8, 2026
Merged

lukemaj merged 1 commit into
fork/v2from
feat/171-issues-v2

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What: GitHub Issues work again on fork/v2: browsing, links to threads, computed status, and opening Issue links in the app, now built on upstream's GitHub client and v2 threads.
Why: Upstream's orchestration v2 removed the gh CLI client and the v1 thread reads these fork features used, and upstream has no Issues feature of its own.
So what: Focused proof, fresh code CI and independent exact-head review pass. GitHub reports this PR merged by lukemaj during final handoff; the dispatcher issued no merge command. UI evidence remains pending the planner's integrated pass, as explicitly agreed.

Problem

The fork's Issues features (#25 and follow-ups) read GitHub through the gh CLI and read threads from the v1 projection. On fork/v2 neither exists, so Issues browse, thread links, computed status and in-app Issue links do not build or run. Thread links stored by v1 Chromeria (fork_thread_issue_links) must still show after the user's data moves to the v2 database.

Change

The fork's existing implementation is ported with the smallest edits to upstream files. Behavior stays the same except for the user decisions listed below.

  • GitHub client: gh calls are replaced by upstream's GitHubApi. Search text, URLs and numbers travel as GraphQL variables. Batched reads use the shared aliasedGraphQlDocument, and the Issues search uses readGraphQlPages with from set to the client's cursor and maxPages: 1, so each call still makes one request and "Load more" works as before. Each Issues list uses one credential per host for all its searches. The closing-reference cache is keyed by credential, as upstream's PR caches are. The server reads only hosts that hold one of its GitHub projects.
  • Fork checkouts: Issues use the checkout's own repository (identity.origin, otherwise the canonical one), so a fork lists its own Issues (D8a). PR lookup is unchanged.
  • Thread reads: a single thread's context comes from OrchestratorV2.getThreadShell and ProjectService. Reads across many threads (threads for Issues, pull requests of Issue threads, child threads, pruning) query the live v2 projection table, the way upstream's pullRequest/linkedThreads.ts does. They never read the frozen v1 tables.
  • Child threads: links roll up only through unbroken subagent lineage (native parentThreadId). Conversation forks and anything below them are excluded. This matches v1, which rolled up only the threads its threads toolkit spawned.
  • Working status: derived from the v2 shell (runtime.status, plus background subagent/background_task work). A spawned thread's work counts for its ancestors.
  • MCP tools: link_issue/unlink_issue use McpToolAccess.actsAsCaller and list_thread_issues uses readsAsCaller, all behind the pull-requests capability.
  • Web: the Issues view on the pull requests page, the sidebar and command palette entries, the "Linked PRs and Issues" panel (child threads' PRs and Issues included), an Issue tab beside the thread (which Cmd/Ctrl+Shift+T reopens), and Issue links in chat. Where a server lists no Issues, /issues/N links keep upstream's behavior, including the PR panel's not-found state (#14242).
  • Inventory: docs/fork-features.md adds four features: issues-browse, issues-links, issues-status and issues-open-links. This PR edits 28 upstream files. scripts/fork-upstream-edits.txt adds 25 of them, each with exactly one owner in these features. The other three, server.ts, ws.ts and SidebarChrome.tsx, keep their existing owners and are listed as shared.

Permission change (user decision D27)

Commenting on, closing or reopening an Issue now requires source-control:write, the scope upstream uses for pull request writes. v1 required orchestration:operate. Both the server and the client guard (CLIENT_GUARDED_RPC_SCOPES, plus the command's permissionAtom in the Issue panel) check the new scope.

  • A grant with orchestration:operate but without source-control:write can no longer comment on, close or reopen Issues. It can still read them and link them to threads.
  • A grant with source-control:write but without orchestration:operate can now comment on, close and reopen them.
  • No grant is expanded automatically. Standard pairings include both scopes, so default clients see no change.
  • To give a client the permission, create a fresh pairing link that includes it, for example npx t3 pair --scope orchestration:read --scope orchestration:operate --scope source-control:write, or a standard pairing from Settings → Connections. Open the link in the browser, or use Add Environment for a saved remote or mobile environment; pairing the same environment replaces its saved grant. Reconnecting alone does not change permissions. See Manage or revoke access; the Issues section of docs/user/source-control.md now says this.

Stored links across the v1 import

This PR adds no write backfill. The v2 database starts as a copy of v1, so fork_thread_issue_links is carried over unchanged, and the importer keeps thread ids and deletion times. Startup still runs the fork backfill hook (runForkV1Backfills() after reconcileShells), which is idempotent here. The proof that the hook recovers from a failed step without duplicate rows is #167's runner test (forkV1Backfills.test.ts), not this PR.

Scope and approval

  • Port 5 of 11 under the per-feature port plan in Absorb upstream main (533 commits behind) #166 (D1–D7, D7.7 "Issues features: Port", D8a origin-first Issues). This PR closes Port the GitHub Issues features #171.

  • D27 (user): source-control:write for Issue writes, and the shared pager with from + maxPages: 1.

  • Upstream checked first: main has no Issues feature. Two open PRs overlap and were inspected but not adopted (findings on #171):

    • pingdotgg/t3code#6315: multi-tracker Issues stored in upstream's v2 thread payload. It is unmerged and its scope is not maintainer-approved. Its MCP tool names collide with ours.
    • pingdotgg/t3code#16552: a draft read-only Issue tab. It uses the same issues.detail RPC name as ours.

    Both collisions need a decision at the next upstream absorption if either PR merges.

  • Not ported:

    • issueStatus.live.test.ts: it creates branches and workflows on GitHub through gh.
    • IssueLinks.live.test.ts: its closing-reference read now lives in IssueService.live.test.ts.
    • ThreadLinksPanel.test.tsx: a static-markup test, which AGENTS.md rules out; logic tests cover its state mapping.

Verification

Candidate 8b7ce7b7c979d1b79271339799dbcac9520a9404 (one commit) on base origin/fork/v2 6e1653b6719565db2cc5094670e19e6efd3fc1ce (#178, squash of the #167 foundation). The upstream merge base is 12069eefd707.

Environment: macOS, Node v24.13.1, the host-neutral environment from docs/fork.md (Homebrew removed from PATH, ELECTRON_RUN_AS_NODE unset, resolved TMPDIR). Script and logs: /tmp/171-v2/final-8b7ce7b7c9/ (run.sh, results.txt, one log per step). Every step exited 0.

The previous head 17ec0845ed failed CI's knip check on one unused export, CREATED_AT in apps/server/src/issueLinks/IssueLinks.testFixtures.ts. It is used only inside that file, so only its export keyword was removed; the commit is otherwise identical. Every step below was rerun on the new head. Fresh CI passes, including knip, full test shards, typecheck, build and release smoke; Fork Stack Model passes. Preview deployment/EAS and unchanged-mobile analysis are conditional workflow skips, not executed UI or live-write proof. The current-head bot transfer report passes every ceiling; it has no successful main baseline yet. No unresolved bot finding remains.

Step Command Result
Fork stack scripts/fork-check.sh OK. Base 12069eefd7, 2 commits (foundation + this PR), all 86 upstream edits in the stack allowlisted with one owner each, 12 features.
Issues tests vp test run apps/server/src/issues apps/server/src/issueLinks packages/client-runtime/src/state/issueCommandPermissions.test.ts packages/contracts/src/issue{s,Links,Status}.test.ts apps/web/src/components/issues apps/web/src/components/threadDescendants.logic.test.ts apps/web/src/components/pullRequest/pullRequestFilterSearch.logic.test.ts 207 passed, 10 skipped; see the skip audit below.
Tests next to the edited upstream files vp test run on 21 files: RpcAuthorization, ServerEnvironment, McpHttpServer, worktree registration, RpcInstrumentation, ws, ChatMarkdown (3 files), ChatView.logic, CommandPalette.logic, RightPanelTabs (2 files), PullRequestListFilters, openPullRequestLink, reopenClosedView, rightPanelStore, state/pullRequests, client-runtime rpc client, contracts environment and rpc 518 passed, 0 skipped
Real-data carryover CHROMERIA_V1_SNAPSHOT_SOURCE=$HOME/.t3/userdata/state.sqlite vp test run apps/server/src/issueLinks/IssueLinks.carryover.test.ts 2 passed (1 synthetic, 1 real). Details below.
Live GitHub, read-only T3CODE_ISSUES_LIVE=1 vp test run apps/server/src/issues/IssueService.live.test.ts (with gh on PATH) 8 read-only tests passed; the 1 write fixture test stayed skipped and did not run.
Typecheck tsc --noEmit in packages/contracts, packages/client-runtime, apps/server, apps/web exit 0, 0 errors
Lint vp lint on all 85 changed TS/TSX files exit 0. The 27 edited upstream TS/TSX files have 145 warnings at base and 145 at head, with identical rule and message, so none are introduced. The new fork files have no warnings.

Real-data carryover. The source is opened read-only and copied with VACUUM INTO. The real importer and the startup hook then run on the private copy. The live threads' stored rows, with every persisted field (thread_id, host, repository, number, url, source, linked_at), are deep-equal (isDeepStrictEqual) to the v1 rows before the import, after the reads and prune, and after a repeated reconcileShells + runForkV1Backfills. Every stored link shows on its thread and is found from its Issue. Failure messages print only counts and digests, never rows. The test does not report the size of the snapshot it compared. Separately, a read-only count of the source taken just after the run found 163 stored links on 101 live threads, all agent-sourced; that count is not the test's input. Dismissed, manual and started links are covered by the synthetic test.

Skip audit. The default run skips exactly 10 tests, all opt-in by environment variable:

  • 9 in apps/server/src/issues/IssueService.live.test.ts, whose suite is describe.skipIf(!live), where live is T3CODE_ISSUES_LIVE === "1". These are 8 read-only tests and 1 write fixture test. The write test is also skipIf(!hasFixture) and runs only when T3CODE_ISSUES_LIVE_FIXTURE names a disposable Issue. That variable stayed unset, so the write test never ran: no GitHub write happened.
  • 1 in apps/server/src/issueLinks/IssueLinks.carryover.test.ts, the real-data test, which runs only when CHROMERIA_V1_SNAPSHOT_SOURCE is set.

Both files then ran separately with their variables set: 8 live read-only tests passed and the write test stayed skipped; both carryover tests (1 synthetic, 1 real) passed.

Behavior these tests prove:

  • Permissions (D27):
    • Server (issueRpcAuthorization.test.ts): an operate-only grant is refused on comment and close with requiredPermission: source-control:write, before the handler runs. A source-control-only grant reaches both handlers.
    • Client (issueCommandPermissions.test.ts): with operate only, permissionAtom is false and authorize fails; with source-control only, it is true and authorize passes.
  • Paging (D27): the first call makes one request with after: null and returns the cursor as nextCursors; the continuation makes one request with that cursor.
  • Credentials: one credential is pinned per host across a list's searches. Closing references are kept per account.
  • Lineage: root → fork → subagent and root → subagent → fork → subagent both stop at the fork, in the server roll-up, the web descendants and the working status.
  • MCP access: the pull-requests capability is required, a caller from outside a T3 thread is refused, and link, list and unlink act on the caller's own thread.
  • Issue tab and fallback: opening, replacing and reopening the Issue tab, and links falling back to upstream handling where a server lists no Issues.

Test rewritten: upstream's RightPanelTabs.keyboard.test.tsx used the menu label "Linked pull requests" to show that the Mod+T menu had opened. The ported label is "Linked PRs and Issues", so only that string changed. All the keyboard, focus and selection assertions are unchanged, and the file is allowlisted under issues-links.

Not verified:

  • UI: before/after images are pending the parent's integrated pass of the UI on fork/v2. Browser use was not authorized for this port; the focused logic tests above prove the behavior in scope.
  • Mobile: there is no Issues UI on mobile, the same as v1.
  • Imported v1 child threads: they arrive with a null parent, so their links reach the parent's panel only after Port child threads onto upstream lineage and delegation #168's lineage repair.
  • Live GitHub writes: not run.
  • Repo-wide checks: left to CI.

Elon record

Closes #171

Independent review: passed. Final findings: no actionable defect after full diff and consumer review. Prism selected Codex / gpt-6-luna, effective effort max. Newest review/independent status is success on exact head 8b7ce7b7c979d1b79271339799dbcac9520a9404, posted by authenticated lukemaj and independently verified by the dispatcher. The speculative credential finding was withdrawn with installed-source evidence; no hypothetical patch was made. All final code CI jobs pass, including the native fingerprint job that was pending when the reviewer checked. The ready-event size-label metadata job is still running; it does not change the code-proof disposition.
Cost: published. Agent Observer report. Cost remains unknown because response usage is not fully attributed; this is a measurement gap, not zero cost.

Coordination: Codex / gpt-6.1-sol, effective effort medium, inside T3 Code.

Model and harness: Claude Opus 5.5 (claude-opus-5-5) in Claude Code (claudeAgent, inside T3 Code). Effective effort is unknown because the runtime does not report it.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Oct 8, 2026
@lukemaj

lukemaj commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Agent work on this PR

Estimated cost unknown · 0 responses · 40 sessions · 4.5 h wall time

Model Responses Tokens Estimated cost

Flags: 3 human corrections · 59 large tool outputs · 37 repeated commands · 8 repeated failures · 62 repeated reads · 5 repeated skill loads · 39 sessions with usage bound to no task · 1 session without usage records

Details: snapshot, prices, coverage, counters
  • Task toolboxmd/chromeria#171: outcome unknown (recorded acceptance only; a finished process never implies it).
  • Proof: Port the GitHub Issues features #171
  • Snapshot 02f7a8a1806ed9e50a5092bba9ea76531c8bbcdcdd1e0d2161e8d17d907accfa, records up to 2026-10-08 18:16 UTC.
  • 40 sessions on claude, codex; AgentsMD 14.6.0.
  • Not counted: 3,831 responses (at least $54.61) in sessions shared with other PRs that worked in no single PR's checkout.
  • 2,176 responses in these sessions worked on other PRs and are counted there.
  • Totals reconcile with the measured sessions: yes. Evidence complete: no.
  • Prices: list-price estimate from T3 local rate table (path withheld) as of 2026-09-28, schedule 696aae45933d0a684a015d3f08cfb6f1aa2fef02e7d272b4689e3a692ea04fff. Unknown prices stay unknown, never zero.
    • T3 LiteLLM rate table when present; bundled schedule covers the rest.
  • Native session usage or worker ownership is unavailable.
  • Harness-reported cost: none reported.
  • Usage totals are not billing. Subscription spending is separate and is never posted as spend.
  • Crashed runs are counted separately: 0.

Token counters by model (native counter semantics; never added across semantics):

Selected rates (USD per million tokens). These rates value the report at the selected schedule date; they do not establish historical prices or subscription spending.

Model Input tier Input Cache read Cache write Other output Reasoning

Other output and reasoning are priced without double counting inclusive native output. Missing rates remain unknown.

Local measurement from native records; usage totals are not billing. Updated in place by agent-observer publish.

… onto v2 (#171)

Upstream's v2 removed the gh CLI client and the v1 projection reads the fork's
Issues features used. This ports them onto upstream's GitHubApi (GraphQL
variables, the shared pager, credential pinning per host), v2 thread reads,
McpToolAccess and native subagent lineage, keeps fork_thread_issue_links
unchanged across the v1 import, and requires source-control:write for Issue
comment and close (D27). Feature map and allowlist list the four features.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 5.0 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.8 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.9 KiB — 29.3 KiB ✅
Codex Live turn messages — 2 — 8 ✅
Claude Total thread wire — 5.0 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.8 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 21.2 KiB — 29.3 KiB ✅
Claude Live turn messages — 2 — 8 ✅

Baseline: unavailable · PR result: 8b7ce7b · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@lukemaj

lukemaj commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

What: Review of PR #179 at head 17ec0845edeb3967120d6e4df6782a8c3b3df9a6 against base 6e1653b6719565db2cc5094670e19e6efd3fc1ce found a blocking CI lint issue.
Why: Lint job 113459222469 reports CREATED_AT as an unused export at IssueLinks.testFixtures.ts:24; the exact-head source uses it only within that fixture file.
So what: Remove the unused export and rerun proof on the successor SHA. This finding applies only to the reviewed head.

Exact-head finding

  • PR: feat(fork): port GitHub Issues onto v2 #179
  • Base: 6e1653b6719565db2cc5094670e19e6efd3fc1ce
  • Head: 17ec0845edeb3967120d6e4df6782a8c3b3df9a6
  • Verdict: failure for this SHA because the required CI lint job fails.
  • Required: IssueLinks.testFixtures.ts:24 exports CREATED_AT, but the CI unused-code check finds no importing consumer. Its references are all in the fixture module. The Lint job in run 37820275401 ends with:
    Unused exports (1): CREATED_AT apps/server/src/issueLinks/IssueLinks.testFixtures.ts:24:14.
  • Effect: The CI lint gate cannot pass on this candidate. The author has a one-line removal in a local successor candidate; this review does not modify source.

Reviewer and limits

Reviewer runtime: GPT-6-Luna (gpt-6-luna) via the Codex harness; runtime metadata exposes max reasoning effort. The provider field is not separately exposed. I verified the exact source at the reviewed SHA, the CI job log, and that gh pr diff matches the local base-to-head diff byte for byte. This comment settles the known blocker on the old SHA; full independent code review continues against the successor candidate. I did not run tests, start a server, use a browser, or perform live GitHub mutations. Integrated UI verification and before/after images remain explicitly deferred to the planner's later pass.

Elon record

Requirements and who asked: #171 and #166 D7.7 require the Issues port; the user requested independent review of the exact PR head.
Deleted: No candidate PR implementation, broadened feature scope, browser pass, or reviewer-run test was needed to establish this finding.
Bottleneck: The exact-head CI unused-code failure prevents this candidate from passing its lint gate.
Checked myself: Exact old source, full CI lint diagnostic, live PR base/head, and byte-identical GitHub/local diffs.

@lukemaj

lukemaj commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

What Independent review finds a blocking P2 multi-host credential isolation defect at head 8b7ce7b7c979d1b79271339799dbcac9520a9404.
Why One host's credential lookup fails issues.list before the service can return successful host rows and per-host errors.
So what Isolate unavailable hosts, then request fresh exact-head review; this review records failure and does not authorize merge.

Finding: P2, blocks the accepted partial-host behavior

apps/server/src/issues/IssueService.ts:285 yields pinnedCredentials for every searched and linked-only host before entering the per-search Effect.catch at lines 338-339. If any one host has no credential, is disabled, or cannot provide one, that yield fails the entire RPC. For example, with valid github.com auth and an unconfigured ghe.example project, an unfiltered issues.list({state:"open"}) loses the GitHub.com results too. A filtered search can also fail because a different host is pinned only for its supplemental linked-PR read.

The expected partial-host contract is explicit in packages/contracts/src/issues.ts:147: failed hosts are reported in errors, while the others still answer. Record credential failures per host so successful searches return their rows and an error for the failed search; an unavailable linked-only read should not abort the Issue list.

Review record

  • Base: fork/v2 6e1653b6719565db2cc5094670e19e6efd3fc1ce
  • Head: feat/171-issues-v2 8b7ce7b7c979d1b79271339799dbcac9520a9404
  • Verdict: failure, due to the P2 finding above.
  • Scope: exact PR diff against the stated base; successor delta from 17ec0845edeb3967120d6e4df6782a8c3b3df9a6 only removes the unused fixture export.
  • Reviewer: Codex harness, GPT-6-Luna, max effort.
  • Limits: I inspected the supplied focused proof logs but did not run tests, live GitHub operations, or browser checks. The integrated UI pass was explicitly deferred by the user. CI for this pushed head was still running when this finding was recorded. Remaining independent diff review continues.

Requirements and who asked: User-approved #171 port of existing Issues browse, links, computed status, and in-app open on native v2; accepted #166 D7.7/D8a and fixed D27 apply.
Deleted: No scope expansion; no candidate PR code or upstream version change is part of this finding.
Bottleneck: Exact-head review found one service-level host failure that defeats the partial-host contract.
Checked myself: Confirmed the credential pin call, per-search catch placement, result contract, exact base/head, and focused proof artifacts.

@lukemaj

lukemaj commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

What The reported multi-host issues.list credential finding is withdrawn.
Why The installed Effect source confirms Effect.cached creates a lazy wrapper, so credentials are acquired under the later per-host catches.
So what No repair is requested for this path; the exact-head review remains pending while I finish.

Correction

The earlier finding in comment #6066102195 was incorrect. I verified the installed source at apps/server/node_modules/effect/src/internal/effect.ts:4539-4540: cached returns sync(() => makeCachedUnsafe(self, infiniteTTL)[0]). The public docs at apps/server/node_modules/effect/src/Effect.ts:13896-13914 say it lazily computes the result. Therefore yield* pinnedCredentials(...) constructs cached wrappers; it does not run api.credential there. The cached effect is evaluated inside the per-search handler and its Effect.catch at apps/server/src/issues/IssueService.ts:305-339; linked-only reads evaluate it inside their catch at lines 347-370. The missing-host credential scenario I reported is caught as intended, so it does not establish a partial-host defect.

This correction supersedes the finding; the exact-head review is still in progress.

Requirements and who asked: User-approved #171 port of Issues browse, links, status, and in-app open on native v2; accepted #166 D7.7/D8a and fixed D27 apply.
Deleted: The false credential lookup finding; no implementation change is requested.
Bottleneck: Complete the remaining exact-head code review.
Checked myself: Read the installed Effect implementation and docs, then traced cached-effect execution through both per-host catches on the exact PR head.

Reviewer: Codex harness, GPT-6-Luna, max effort. No tests or browser were run by reviewer.

@lukemaj

lukemaj commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent review of PR #179 at 8b7ce7b7c979d1b79271339799dbcac9520a9404 found no actionable code defect.
Why: The exact-head diff ports the accepted Issues behavior into native v2; the credential lookup concern is disproved by the installed Effect implementation's lazy cached-effect semantics.
So what: I am recording review/independent success on this exact head; the integrated UI pass remains deferred as requested, and the parent owns CI monitoring.

Exact candidate and verdict

  • PR: feat(fork): port GitHub Issues onto v2 #179
  • Base: fork/v2 at 6e1653b6719565db2cc5094670e19e6efd3fc1ce
  • Head reviewed: feat/171-issues-v2 at 8b7ce7b7c979d1b79271339799dbcac9520a9404
  • Verdict: no supported actionable finding in the inspected scope. The 8b7 head's only change from the previously reviewed candidate removes the unnecessary CREATED_AT export from IssueLinks.testFixtures.ts.

Resolved credential concern

The earlier claim in comment #6066102195 is withdrawn in the public correction. The installed Effect.cached implementation (apps/server/node_modules/effect/src/internal/effect.ts:4539) returns sync(() => makeCachedUnsafe(self, infiniteTTL)[0]); its API docs (Effect.ts:13896-13914) also say it computes lazily. IssueService.ts:285-339 builds cached effects, then invokes the credential-bound read inside pinned(host, read) under the per-search catch; linked-only reads have their own catch at lines 344-370. Therefore building the pins does not call api.credential, and a missing-host credential remains isolated to that host's caught read. No code change is warranted.

Review coverage and evidence

I read the full PR diff against the exact base, including the server, contracts, client, UI, tests, documentation, fork inventory, and affected consumers. I checked the accepted #166 scope and D27 authorization choice: Issue comment/close/reopen remain guarded by source-control:write, with no grant expansion. The review covered v2 project/thread context, origin-aware repository matching, one-page cursor-based GraphQL reads, caller-scoped MCP access, Issue/PR link resolution and fallback, lineage rollup, status derivation, and imported V1 link reconciliation.

I inspected the exact-candidate evidence at Issue #171 comment #6066042049 and its logs under /tmp/171-v2/final-8b7ce7b7c9/: feature tests 207 passed / 10 opt-in skips; adjacent tests 518 passed; real V1 carryover 2 passed; live read-only GitHub 8 passed / 1 mutation skipped; four package typechecks, focused lint, and fork check passed. The upstream-lint comparison has the same 145 baseline warnings on base and head. I did not rerun these checks.

At the time checked, CI Build, Check, Lint, Typecheck, Test, Test Web, Test Server 1-6, Rust, Release Smoke, and Fork Stack Model passed. Native fingerprint diff was still pending; web preview and mobile static analysis were skipped. I am not treating this independent code-review status as a claim that all CI is complete.

Limits

The user explicitly deferred the browser/UI before-and-after pass until the ports are integrated in fork/v2; no browser or client UI pass was run for this review. The live GitHub mutation remained gated and skipped. These are stated proof limits, not code findings or changes to the accepted scope.

Elon record

  • Requirements and who asked: Issue Port the GitHub Issues features #171 and accepted Absorb upstream main (533 commits behind) #166 D7.7/D8a define the Issues browse/link/status/open behavior; the user fixed D27 authorization and deferred visual acceptance.
  • Deleted: No implementation or proof scope was added; the unsupported eager-credential finding was withdrawn after checking the installed Effect source.
  • Bottleneck: Final integrated UI acceptance and remaining CI observation are owned by the parent after the ports.
  • Checked myself: Exact base/head and complete diff, relevant source paths and consumers, installed Effect implementation/docs, supplied exact-head proof logs, and live PR checks.

Reviewer: Codex / GPT-6-Luna, T3 Code harness, max reasoning effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant