Align planting path guards and accepted scope labels

Author: mchwang

docs/implementation/read-only-review.md

@@ -42,3 +42,5 @@ Extended the attribution guard to mixed items with both owned and ambiguous segm
 A toolbar-focus browser regression reproduced review shortcuts being disabled while a button had focus. Only text-entry/select controls now suppress the single-letter shortcuts; native Enter/Space button behavior is unchanged. The plan-row click case already worked because rendering replaces the focused row.
 
 Review round 3 identified demo fixture config escapes and inherited Git environment redirection. Regression checks first reproduced the config, symlink, and environment failures. Existing demos now require exact fixture paths and ordinary config/database/repository objects (including `.git` and SQLite sidecars). Demo and plant Git commands share a case-insensitive environment scrub and disable inherited global/system Git configuration. Four helper tests cover these boundaries, including mixed-case variable names for Windows.
+
+Review round 4 had no inline findings but raised two summary concerns. Both were reproduced: planting accepted a non-ASCII path under case-insensitive identity even though the viewer refuses it; accepted cards retained an unplanned scope label. Planting now applies the same path guard, and accepted cards explicitly say “Accepted outside plan.” The underlying provenance remains intact. Unit/browser regressions cover both fixes; no concerns were declined.

scripts/plant.ts

@@ -16,10 +16,17 @@ export function plant(config: ReviewConfig, destination: string, input: PlantInp
   const root=resolve(destination);
   if(existsSync(root))throw new Error('Experiment destination must not exist.');
   if((!isRepoPath(input.undeclaredPath)||input.undeclaredPath.includes('/'))||!input.declaredText?.trim()||!input.undeclaredText?.trim()||input.declaredText.length>4000||input.undeclaredText.length>4000)throw new Error('Invalid plant input.');
+  const pathKey=(path:string)=>{
+    if(!config.pathIdentity.caseSensitive && /[^\x20-\x7e]/.test(path)) throw new Error('Non-ASCII case-insensitive paths require a filesystem-specific identity adapter.');
+    const p=config.pathIdentity.unicodeNormalization==='NFC'?path.normalize('NFC'):path;
+    return config.pathIdentity.caseSensitive?p:p.toLowerCase();
+  };
+  pathKey(input.undeclaredPath);
   const source=new Store(config.database);
   let plan, snapshot, entries;
   try{plan=source.getPlan(config.identity);snapshot=source.getSnapshot(config.identity);entries=source.getLedger(config.identity);}finally{source.close();}
   if(plan.items.some(item=>item.files.some(file=>file.path===input.undeclaredPath||file.renamed_from===input.undeclaredPath)))throw new Error('Undeclared plant must be outside every declared file.');
+  for(const item of plan.items) for(const file of item.files) { pathKey(file.path); if(file.renamed_from) pathKey(file.renamed_from); }
   const history=readHistory(config.repository,snapshot.base,snapshot.head);
   const owners=new Map(entries.map(entry=>[entry.sha,entry.owner]));
   const candidates=history.commits.flatMap((commit,index)=>{
@@ -64,7 +71,6 @@ export function plant(config: ReviewConfig, destination: string, input: PlantInp
     const output:ReviewConfig={...config,repository,database:join(root,'review.sqlite'),identity,demo:false};
     const store=new Store(output.database);
     try{
-      const pathKey=(path:string)=>{const p=config.pathIdentity.unicodeNormalization==='NFC'?path.normalize('NFC'):path;return config.pathIdentity.caseSensitive?p:p.toLowerCase();};
       store.createPlan(JSON.stringify(plan),'json',{identity,issue:plan.issue,baseEntries,pathKey,allowedCommands:[]},snapshot.base,snapshot.head);
       const validEntries=entries.filter(entry=>entry.owner===null||plan.items.some(item=>item.id===entry.owner));
       store.recordHistory(identity,{revision:1,snapshotId:store.getSnapshot(identity).id},snapshot.base,snapshot.head,validEntries);

test/browser/review.spec.ts

@@ -63,7 +63,7 @@ test('keeps stale item controls unavailable after a failed refresh',async({page}
  await page.getByRole('button',{name:'Refresh',exact:true}).click();await expect(page.getByText('Could not read this branch’s history.',{exact:false})).toBeVisible();await expect(page.getByRole('button',{name:/P3 Confirm API compatibility/})).toHaveCount(0);
 });
 test('accepts a foreign segment and keeps that choice across reload',async({page})=>{
- await page.goto(app.url);await page.getByRole('button',{name:/Unplanned changes/}).click();await page.getByRole('button',{name:'Accept as is',exact:true}).first().click();await page.getByRole('button',{name:/Accepted 1/}).click();await expect(page.getByRole('article').first()).toBeVisible();await page.reload();await page.getByRole('button',{name:/Accepted 1/}).click();await expect(page.getByRole('article').first()).toBeVisible();
+ await page.goto(app.url);await page.getByRole('button',{name:/Unplanned changes/}).click();await page.getByRole('button',{name:'Accept as is',exact:true}).first().click();await page.getByRole('button',{name:/Accepted 1/}).click();await expect(page.getByRole('article').first()).toBeVisible();await expect(page.getByText('Accepted outside plan',{exact:true})).toBeVisible();await page.reload();await page.getByRole('button',{name:/Accepted 1/}).click();await expect(page.getByRole('article').first()).toBeVisible();
 });
 test('shows the whole-plan empty state without claiming checks passed',async({page})=>{
  const {repository,identity}=app.service.config;const base=app.service.store.getSnapshot(identity).base;execFileSync('git',['reset','--hard',base],{cwd:repository,stdio:'pipe'});

test/plant.test.ts

@@ -15,3 +15,8 @@ it('plants in an isolated clone, retains ledger attribution, and leaves source h
  try {const view=service.load();expect(view.segments.some(s=>s.path==='extra.txt'&&s.scope==='out-of-scope')).toBe(true);expect(view.segments.some(s=>s.content.includes('// planted extra behavior')&&s.row.startsWith('P'))).toBe(true);}finally{service.close();}
  expect(JSON.parse(readFileSync(join(root,'experiment','sealed.json'),'utf8')).mappings).toHaveLength(3);
 },15000);
+it('refuses non-ASCII plants on case-insensitive filesystems pending an identity adapter', () => {
+ const root=mkdtempSync(join(tmpdir(),'codeboost-plant-path-'));roots.push(root);
+ const config=createDemo(join(root,'source'));config.pathIdentity.caseSensitive=false;
+ expect(()=>plant(config,join(root,'experiment'),{declaredText:'// extra',undeclaredText:'diagnostic',undeclaredPath:'café.txt'})).toThrow(/filesystem-specific identity adapter/);
+},15000);

web/public/app.js

@@ -268,7 +268,7 @@ function renderCode() {
         const choices = ["Unplanned", "Ambiguous"].includes(segment.row)
           ? `<div class="choice-controls"><p>Assigning this change makes the selected item’s approval stale.</p><select aria-label="Assign change ${index + 1} to" data-target="${index}"><option value="">Assign to…</option>${data.items.map((p) => `<option value="${esc(p.id)}">${esc(p.id)} · ${esc(p.title)}</option>`).join("")}</select><button data-assign="${index}">Assign</button><button data-accept="${index}">Accept as is</button></div>`
           : "";
-        return `<article class="change ${index === change ? "active" : ""}" data-change="${index}" aria-label="Change ${index + 1}, ${esc(segment.path)}, ${esc(provenance)}"><div class="file-heading"><code>${esc(segment.path)}</code><span class="${segment.scope === "out-of-scope" ? "bad" : "muted"}">${segment.scope === "out-of-scope" ? "✕ Out of scope" : esc(segment.scope)}</span></div>${content}<div class="change-meta">${esc(segment.context || "File-level change")}${segment.sharesHunkWith.length ? ` · Shares a hunk with ${esc(segment.sharesHunkWith.join(", "))}` : ""}</div>${choices}</article>`;
+        return `<article class="change ${index === change ? "active" : ""}" data-change="${index}" aria-label="Change ${index + 1}, ${esc(segment.path)}, ${esc(provenance)}"><div class="file-heading"><code>${esc(segment.path)}</code><span class="${segment.scope === "out-of-scope" ? "bad" : "muted"}">${segment.row === "Accepted" ? "Accepted outside plan" : segment.scope === "out-of-scope" ? "✕ Out of scope" : esc(segment.scope)}</span></div>${content}<div class="change-meta">${esc(segment.context || "File-level change")}${segment.sharesHunkWith.length ? ` · Shares a hunk with ${esc(segment.sharesHunkWith.join(", "))}` : ""}</div>${choices}</article>`;
       })
       .join("") ||
       (data.segments.length