You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add an optional, in-page "load a public repo" path to the Mergepath Playground so a user can type a public owner/repo, and the page fetches that repo's recent merged PRs directly from the GitHub REST API (unauthenticated) and replays them under the current draft policy — no gh CLI, no local script, no token.
This complements the existing scripts/policy-sim.sh injection flow rather than replacing it: policy-sim.sh stays the authenticated path for private repos and larger pulls. The two coexist behind the same "Pull live PRs" affordance.
It reverses one explicitly-hardened design constraint (the playground's "no network calls" Non-goal), so the spec change is part of the deliverable, not an afterthought — see the Spec delta section below.
Live data is injected out-of-band: scripts/policy-sim.sh shells out to gh pr list --state merged (scripts/policy-sim.sh:67), maps each PR to {id, title, author, lines, paths}, script-safe-serializes it, and rewrites the <!-- MERGEPATH_INJECT --> marker (mergepath/playground/index.html:9) into <script>window.__PRS = […]</script> in a temp copy. The page never talks to GitHub itself.
On load the page reads that global once: const isLive = Array.isArray(window.__PRS) (index.html:1117), normalizes via normalizePR (index.html:1102), and flips the header badge to live · N vs synthetic · N (index.html:1411).
There is already a "Pull live PRs" button + modal (refreshBtn / refreshModal, index.html:809) whose current job is only to show the copy-paste policy-sim.sh command. That modal is the natural home for the new in-page loader.
Proposed design
Data path — 1 + N unauthenticated requests
The routing sim needs lines (additions+deletions) and paths (changed files) per PR. The REST list endpoint (GET /repos/{owner}/{repo}/pulls) returns neither. But GET /pulls/{n}/files returns both — per-file filename and additions/deletions — so we collapse the fan-out to 1 list request + one /files request per PR (not 2N):
GET /repos/{owner}/{repo}/pulls?state=closed&per_page={N}&sort=updated&direction=desc → filter to merged_at !== null. Yields number, title, user.login, body.
Per merged PR: GET /repos/{owner}/{repo}/pulls/{number}/files?per_page=100 → paths = files.map(f => f.filename), lines = Σ(f.additions + f.deletions).
Author detection mirrors policy-sim.sh: parse Authoring-Agent:\s*([A-Za-z0-9_-]+) from the PR body, else fall back to user.login.
Minimal loader sketch
// --- Public-repo loader (unauthenticated GitHub REST) --------------------constREPO_RE=/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/;asyncfunctionloadPublicRepo(slug,limit=12){if(!REPO_RE.test(slug))thrownewError('Expected owner/repo');const[owner,repo]=slug.split('/');constbase='https://api.github.com';constheaders={Accept: 'application/vnd.github+json'};// 1 request: recent closed PRs (list endpoint has no additions/paths).constlistRes=awaitfetch(`${base}/repos/${owner}/${repo}/pulls?state=closed&per_page=${limit}&sort=updated&direction=desc`,{ headers });if(listRes.status===404)thrownewError('Repo not found or private');if(listRes.status===403)thrownewError(rateLimitMsg(listRes));if(!listRes.ok)thrownewError(`GitHub ${listRes.status}`);constmerged=(awaitlistRes.json()).filter(pr=>pr.merged_at);// N requests: /files yields BOTH paths and additions/deletions.constprs=[];for(constprofmerged){constfilesRes=awaitfetch(`${base}/repos/${owner}/${repo}/pulls/${pr.number}/files?per_page=100`,{ headers });if(!filesRes.ok)continue;// skip a PR we can't sizeconstfiles=awaitfilesRes.json();// one page; >100-file PRs truncateconstm=(pr.body||'').match(/Authoring-Agent:\s*([A-Za-z0-9_-]+)/);prs.push({id: '#'+pr.number,title: pr.title,author: m ? m[1] : (pr.user&&pr.user.login)||'unknown',lines: files.reduce((n,f)=>n+f.additions+f.deletions,0),paths: files.map(f=>f.filename),});}returnprs;}functionrateLimitMsg(res){returnres.headers.get('X-RateLimit-Remaining')==='0'
? 'GitHub rate limit hit (60/hr unauthenticated). Try later or use policy-sim.sh.'
: 'GitHub returned 403.';}
The fetched objects are the same shapepolicy-sim.sh injects, so they flow through the existing normalizePR + textContent render path unchanged — no new innerHTML, no new XSS surface.
Wiring / refactor
Today isLive and samplePRs are consts computed once at init (index.html:1117), and the header-pill block (index.html:1411) runs once inline. To re-render after a runtime fetch, three small changes:
Route rendering through an accessor instead of the init-time const: const currentPRs = () => (livePRs ?? window.__PRS ?? SYNTHETIC_PRS).map(normalizePR).filter(Boolean); where let livePRs = null.
Extract the header-pill block into renderHeaderPill(isLive, n) so it can re-run.
Add the modal handler:
asyncfunctionpullPublicRepo(slug,limit){announce('Loading '+slug+'…');// existing aria-live regiontry{livePRs=awaitloadPublicRepo(slug,limit);renderHeaderPill(true,livePRs.length);renderAll();// existing full re-renderannounce('Loaded '+livePRs.length+' merged PRs from '+slug);}catch(e){announce('Could not load '+slug+': '+e.message);// keep current view}}
UI: add an owner/repo text input + a small limit control to the existing refreshModal, alongside (not replacing) the policy-sim.sh command block.
Spec delta — specs/mergepath_playground.md
Acceptance criterion Add multi-identity AI agent code review policy #1 — change "single self-contained HTML file with no network, build, or runtime dependencies" → "…no build or runtime dependencies, and no network calls on load; the only network access is an explicit, user-initiated, unauthenticated, read-only fetch of a public repo's merged PRs."
Acceptance criteria (add) — the refresh modal exposes an owner/repo input; a valid slug fetches merged PRs client-side, flips the badge to live · N, and re-renders; invalid slug, 404 (private/missing), 403 (rate limit), and network failure each surface via the aria-live region and leave the current view intact.
Non-goals — keep the write ban and the "no server/proxy" stance; narrow the blanket network ban to carve out the explicit public read. Explicitly keep out of scope: any PAT/token field, private-repo fetch, and any server/proxy component (private repos remain policy-sim.sh's job).
Hardening (add) — owner/repo validated against REPO_RE and length-capped before URL interpolation; unauthenticated request budget surfaced from X-RateLimit-Remaining on 403; N capped; single-page /files truncation for >100-file PRs is documented (lines undercount, some paths dropped) and acceptable; fetched data reuses normalizePR + textContent (no new innerHTML); fetch failure degrades gracefully.
Acceptance criteria
Refresh modal has an owner/repo input; valid public slug loads merged PRs via unauthenticated REST and re-renders under the current knobs.
Badge flips to live · N; synthetic/live and the sim hint update at runtime (no reload).
lines and paths are correct (derived from /files); author honors Authoring-Agent: then user.login.
Invalid slug, 404, 403 (rate-limited), and network error each announce via the aria-live region and keep the prior view.
owner/repo validated + length-capped before interpolation; N capped; no new innerHTML.
specs/mergepath_playground.md updated per the Spec delta; tests/test_mergepath_playground.sh extended to cover the loader + validation.
policy-sim.sh still works unchanged (authenticated / private-repo path).
Open decisions
Default and max N for the unauthenticated path (proposed: default 12, max 15 — keeps a load at 1+N ≈ 13–16 requests, ~4 loads/hr against the 60/hr cap).
Ship an optional PAT field for private repos / higher limits, or leave that to policy-sim.sh? (Proposed: out of scope here — a token in the browser is a separate, larger security surface; file as a follow-up if wanted.)
Add a connect-src 'self' https://api.github.com meta CSP as defense-in-depth? The page has no CSP today. (Proposed: optional, nice-to-have.)
Paginate /files for >100-file PRs at the cost of extra requests against the 60/hr budget? (Proposed: no — one page, document the undercount.)
Notes / risks
Rate limit. Unauthenticated REST is 60 requests/hr per IP. At 1+N per load that's ~4 loads of 12 PRs per hour; a shared/NAT'd IP drains it faster. This is the core reason the authenticated policy-sim.sh path must stay.
Truncation. The single-page /files cap means PRs with >100 changed files undercount lines and drop some paths, so their routing verdict can be slightly off. Acceptable for a playground; documented, not silently wrong.
Security posture. No token in the page is the whole point of the public path — nothing to leak. Reusing normalizePR + textContent means fetched titles/paths inherit the existing XSS hardening.
Surface sensitivity. This edits an explicitly-hardened, spec-governed surface under mergepath/ and specs/. The spec and tests/test_mergepath_playground.sh must change in the same PR as the code.
What to build
Add an optional, in-page "load a public repo" path to the Mergepath Playground so a user can type a public
owner/repo, and the page fetches that repo's recent merged PRs directly from the GitHub REST API (unauthenticated) and replays them under the current draft policy — noghCLI, no local script, no token.This complements the existing
scripts/policy-sim.shinjection flow rather than replacing it:policy-sim.shstays the authenticated path for private repos and larger pulls. The two coexist behind the same "Pull live PRs" affordance.It reverses one explicitly-hardened design constraint (the playground's "no network calls" Non-goal), so the spec change is part of the deliverable, not an afterthought — see the Spec delta section below.
Current state
mergepath/playground/index.html) is a single self-contained HTML file that today makes zero network calls — enforced byspecs/mergepath_playground.mdNon-goals ("Writing to … any server", plus acceptance criterion Add multi-identity AI agent code review policy #1: "no network, build, or runtime dependencies").scripts/policy-sim.shshells out togh pr list --state merged(scripts/policy-sim.sh:67), maps each PR to{id, title, author, lines, paths}, script-safe-serializes it, and rewrites the<!-- MERGEPATH_INJECT -->marker (mergepath/playground/index.html:9) into<script>window.__PRS = […]</script>in a temp copy. The page never talks to GitHub itself.const isLive = Array.isArray(window.__PRS)(index.html:1117), normalizes vianormalizePR(index.html:1102), and flips the header badge tolive · Nvssynthetic · N(index.html:1411).refreshBtn/refreshModal,index.html:809) whose current job is only to show the copy-pastepolicy-sim.shcommand. That modal is the natural home for the new in-page loader.Proposed design
Data path — 1 + N unauthenticated requests
The routing sim needs
lines(additions+deletions) andpaths(changed files) per PR. The REST list endpoint (GET /repos/{owner}/{repo}/pulls) returns neither. ButGET /pulls/{n}/filesreturns both — per-filefilenameandadditions/deletions— so we collapse the fan-out to 1 list request + one/filesrequest per PR (not 2N):GET /repos/{owner}/{repo}/pulls?state=closed&per_page={N}&sort=updated&direction=desc→ filter tomerged_at !== null. Yieldsnumber,title,user.login,body.GET /repos/{owner}/{repo}/pulls/{number}/files?per_page=100→paths = files.map(f => f.filename),lines = Σ(f.additions + f.deletions).Author detection mirrors
policy-sim.sh: parseAuthoring-Agent:\s*([A-Za-z0-9_-]+)from the PR body, else fall back touser.login.Minimal loader sketch
The fetched objects are the same shape
policy-sim.shinjects, so they flow through the existingnormalizePR+textContentrender path unchanged — no newinnerHTML, no new XSS surface.Wiring / refactor
Today
isLiveandsamplePRsareconsts computed once at init (index.html:1117), and the header-pill block (index.html:1411) runs once inline. To re-render after a runtime fetch, three small changes:const currentPRs = () => (livePRs ?? window.__PRS ?? SYNTHETIC_PRS).map(normalizePR).filter(Boolean);wherelet livePRs = null.renderHeaderPill(isLive, n)so it can re-run.UI: add an
owner/repotext input + a small limit control to the existingrefreshModal, alongside (not replacing) thepolicy-sim.shcommand block.Spec delta —
specs/mergepath_playground.mdowner/repoinput; a valid slug fetches merged PRs client-side, flips the badge tolive · N, and re-renders; invalid slug, 404 (private/missing), 403 (rate limit), and network failure each surface via thearia-liveregion and leave the current view intact.policy-sim.sh's job).owner/repovalidated againstREPO_REand length-capped before URL interpolation; unauthenticated request budget surfaced fromX-RateLimit-Remainingon 403;Ncapped; single-page/filestruncation for >100-file PRs is documented (lines undercount, some paths dropped) and acceptable; fetched data reusesnormalizePR+textContent(no newinnerHTML); fetch failure degrades gracefully.Acceptance criteria
owner/repoinput; valid public slug loads merged PRs via unauthenticated REST and re-renders under the current knobs.live · N;synthetic/liveand the sim hint update at runtime (no reload).linesandpathsare correct (derived from/files); author honorsAuthoring-Agent:thenuser.login.owner/repovalidated + length-capped before interpolation;Ncapped; no newinnerHTML.specs/mergepath_playground.mdupdated per the Spec delta;tests/test_mergepath_playground.shextended to cover the loader + validation.policy-sim.shstill works unchanged (authenticated / private-repo path).Open decisions
Nfor the unauthenticated path (proposed: default 12, max 15 — keeps a load at 1+N ≈ 13–16 requests, ~4 loads/hr against the 60/hr cap).policy-sim.sh? (Proposed: out of scope here — a token in the browser is a separate, larger security surface; file as a follow-up if wanted.)connect-src 'self' https://api.github.commeta CSP as defense-in-depth? The page has no CSP today. (Proposed: optional, nice-to-have.)/filesfor >100-file PRs at the cost of extra requests against the 60/hr budget? (Proposed: no — one page, document the undercount.)Notes / risks
policy-sim.shpath must stay./filescap means PRs with >100 changed files undercountlinesand drop somepaths, so their routing verdict can be slightly off. Acceptable for a playground; documented, not silently wrong.normalizePR+textContentmeans fetched titles/paths inherit the existing XSS hardening.mergepath/andspecs/. The spec andtests/test_mergepath_playground.shmust change in the same PR as the code.