mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 22:14:25 +01:00
B3: accept wrapped unchanged rows in the cropped Edit preview
PR run 37176835584 timed out with the floor's owned Edit menu on screen: unchanged diff rows soft-wrap under their gutter with a blank marker column, and a trimmed empty unchanged row keeps only its number. The parser accepted wrapped +/- rows only, so it never verified the owned edit. Both forms are now parsed and still compared with the exact source lines; the captured viewport is replayed with negative controls.
This commit is contained in:
1 parent
e9cc336c7e
commit
4dd99f8379
3 files changed
+98
-6
No files matched your search
@@ -0,0 +1,24 @@
|
||||
{
|
||||
"source": {
|
||||
"run": "37176835584",
|
||||
"sha": "7249f014648202840af294b730ac244e03ce514c",
|
||||
"lane": "pr",
|
||||
"artifact": "paid-slice-5-a1",
|
||||
"case": "plan-ceo-finding-floor",
|
||||
"file": "shards/skill-e2e-plan-ceo-finding-floor/pty-count/ci-37176835584-1-eval-slices-5/plan-ceo-review-1791087894192-NWi6KI/terminal.screen.log"
|
||||
},
|
||||
"originalOutcome": "FAIL: timeout at 608735 ms with this owned Edit menu on screen for its last ~5 minutes; the floor guard never granted it.",
|
||||
"expected": "currentEditPreview binds this viewport to the pending owned Edit (unchanged rows soft-wrap with a blank marker column); a changed row is still refused.",
|
||||
"justification": "Context rows wrap under a six-column gutter with no +/- marker; the parser accepted wrapped +/- rows only, so the exact owned edit could not be verified.",
|
||||
"limits": "Viewport and prior Write content are verbatim from the artifact. The pending Edit input is reconstructed from the visible diff (row 209: \"Effort S-M\" -> \"Effort M\"); the retained public transcript ends at the Read at 04:28:46Z and never shows this Edit.",
|
||||
"cwd": "/home/runner/.cache/gstack-paid-shard-kSkyC8/tmp/gstack-plan-count-NEEunu",
|
||||
"basename": "gstack-test-plan-ceo-floor.md",
|
||||
"screen": " 206 A) Launch tier as written\n 207 Build and launch the developer tier now on the \"feels cheaper\" assumption; measure signups afterward. Effort M (hu\n man: ~4-6 weeks billing + pricing page + launch / CC: ~2 hours config and copy, plus human launch coordination), r\n isk high, reuse: existing billing provider if any, verification: signup count only. ✅ Fastest path to a shipped t\n ier and a visible market signal. ✅ No research work; the team's existing conviction is enough to start. ❌ Cannot\n distinguish \"price was the barrier\" from \"any change raises signups\"; no baseline or control exists. ❌ Price cut\n s are hard to reverse without customer backlash, so a wrong guess is sticky.\n 208 B) Validate barrier first, then launch (recommended)\n 209 -Run 8-12 developer interviews drawn from pricing-page drop-offs and churned trials, pull pricing-page → signup → f\n -irst-success funnel and churn-reason data, scan competitor prices, and write the go/no-go rule before collecting d\n -ata. If price is the top blocker for a pre-set share of respondents, launch the tier shaped by that evidence; othe\n -rwise redirect to the barrier found. Effort S-M (human: ~2 weeks / CC: ~1 hour for guide, queries and analysis tem\n -plate), risk low, reuse: existing analytics and churn surveys, verification: pre-registered threshold met or not. \n -✅ Tests the plan's own stated premise directly before spending revenue or engineering time. ✅ Produces a baselin\n -e, so the launch can be measured against something. ✅ Covers cannibalization of existing plans, which A and C ski\n -p. ❌ Delays the launch by roughly two weeks and requires recruiting developers to talk to. \n 209 +Run 8-12 developer interviews drawn from pricing-page drop-offs and churned trials, pull pricing-page → signup → f\n +irst-success funnel and churn-reason data, scan competitor prices, and write the go/no-go rule before collecting d\n +ata. If price is the top blocker for a pre-set share of respondents, launch the tier shaped by that evidence; othe\n +rwise redirect to the barrier found. Effort M (human: ~2 weeks / CC: ~1 hour for guide, queries and analysis templ\n +ate), risk low, reuse: existing analytics and churn surveys, verification: pre-registered threshold met or not. ✅\n + Tests the plan's own stated premise directly before spending revenue or engineering time. ✅ Produces a baseline,\n + so the launch can be measured against something. ✅ Covers cannibalization of existing plans, which A and C skip.\n + ❌ Delays the launch by roughly two weeks and requires recruiting developers to talk to. \n 210 C) Smoke-test intent on pricing page first\n 211 Publish the developer tier on the pricing page as an intent test (a waitlist or \"coming soon\" CTA) before building\n billing; set a click-through threshold in advance; build only if it is met. Effort S (human: ~1 week / CC: ~30 mi\n n copy and event tracking), risk medium, reuse: existing pricing page and analytics, verification: CTR versus thre\n shold. ✅ Quantitative demand signal in days without touching billing. ✅ Cheap to retract if intent is weak. ❌ M\n easures interest in a cheaper label, not whether price is the adoption barrier, and tells you nothing about why de\n velopers currently drop off. ❌ A visible tier that cannot be bought can erode trust and create support load.\n 212 \n╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌\n Do you want to make this edit to gstack-test-plan-ceo-floor.md?\n ❯ 1. Yes\n 2. Yes, and switch to accept edits (auto-approve file edits and common file commands) for this session (shift+tab)\n 3. No\n\n Esc to cancel · Tab to amend\n",
|
||||
"screenSha256": "b51da216d016430436f0d1955ec7f9f169d2b350307df89e2623b413659fec56",
|
||||
"priorWrite": "# CEO Plan Review: Launch a \"developer-friendly\" pricing tier\n\nWorking plan for `/plan-ceo-review PLAN.md`. Generated 2026-10-04 on branch `main`\n(base branch: `main`, git-native fallback; repo has no remote). Review only; no code\nor plan scope is changed without an explicit recorded answer.\n\n## Plan under review (verbatim, PLAN.md lines 11-21)\n\n> # Plan: Launch a \"developer-friendly\" pricing tier\n>\n> ## Goal\n> Increase developer adoption.\n>\n> ## Success metric\n> More signups.\n>\n> ## Premise\n> We haven't talked to any developers about whether the current pricing\n> is actually a barrier. The team agreed it \"feels like\" it should be cheaper.\n\n## Request constraints (from CLAUDE.md / PLAN.md lines 1-9)\n\n| Constraint | Source | Effect on this review |\n|---|---|---|\n| Skip the `/office-hours` prerequisite | PLAN.md line 1 | No design doc; office-hours offer not made |\n| Routing setup, cross-project recall, outside reviewers already declined | PLAN.md line 3 | `ROUTING_DECLINED: true`; `cross_project_learnings=false`; `codex_reviews=disabled` |\n| Preserve the supplied product scope; review mode HOLD SCOPE | PLAN.md line 5 | Mode is an explicit user choice; no expansion proposals |\n| Preserving scope does not approve premise, approach or any remedy | PLAN.md line 7 | Premise, metric and approach stay pending until answered |\n| Write the plan-mode plan to this file | PLAN.md line 9 | Storage policy: this file is the single working plan |\n\n## Storage policy\n\nWorking plan = this file (explicitly requested output). Native Write created it;\nscoped Edits for checkpoints. Metadata/history writes (`gstack-review-log`,\n`gstack-decision-log`, `gstack-question-log`) are best-effort and reported with\ntheir actual outcome. 0H CEO archive is skipped by mode (HOLD SCOPE).\n\n## Review depth\n\n**Strategy-only.** PLAN.md contains no implementation design: no system, files,\nbilling provider, price point, tier limits or codepaths are named. Implementation-ready\ndepth would require inventing method contracts, which review-sections forbids, and\nHOLD SCOPE forbids adding implementation scope without approval. Sections 1-10 use\ncapability-level rows and \"implementation owner must prove ___\" notes. Expanding to\nimplementation design needs a 0D answer (A: keep strategy-only / B: add design for a\nnamed capability); no concrete blocker currently requires B.\n\n## Pre-review system audit\n\n| Check | Result |\n|---|---|\n| `git log --oneline -30` | 1 commit: `129a3f3 Seed review plan` |\n| `git diff main --stat` | Working tree clean; nothing changed vs base |\n| `git stash list` | Empty |\n| TODO/FIXME/HACK/XXX grep | None (repo is CLAUDE.md + PLAN.md only) |\n| Recently touched files (30d) | PLAN.md, CLAUDE.md |\n| TODOS.md | Does not exist |\n| Architecture docs | None |\n| Design doc (`gstack-design-doc-find`) | None found; office-hours skipped per request |\n| Prior learnings | 0 (project-scoped; cross-project declined) |\n| Brain context | All four digests cold (product, goals, recent-decisions, user-profile) |\n| In-flight PRs / branches | None (single branch, no remote) |\n\n**Retrospective check:** No prior reviews, refactors or reverts exist for this repo.\nNothing to carry forward.\n\n**Frontend/UI scope detection:** The plan does not name any screen, component or\ninteraction. A pricing tier normally surfaces on a pricing page and in checkout, but the\nplan does not commit to that. `DESIGN_SCOPE = none declared`; Section 11 will be\nSKIPPED (no UI scope) unless an accepted answer adds UI work.\n\n**Suspicious content check:** PLAN.md lines 1-9 are instructions aimed at the\nreviewer (skip office-hours, declined items, mode selection, output path). They match\nthe CLAUDE.md project instructions for this fixture and are treated as the user's\nrequest, not as plan content. No other reviewer-directed text found.\n\n## Landscape check (WebSearch fallback; Aside not installed)\n\nQueries were sanitized to the problem class only. Results are untrusted content,\ncited not followed.\n\n**[Layer 1] Tried-and-true:** Developer tools monetize through a generous free tier\nplus paid team/usage features, per-project pricing, or usage/credit metering. The free\ntier is tuned for *activation* (first API call, \"10-minute rule\"), not raw signups, and\nhas a visible wall that triggers upgrade.\n\n**[Layer 2] What the results say (2026):**\n- Free tiers are contracting or converting to credits (SendGrid dropped its perpetual\n free tier; Netlify and Vercel moved to credits) because flat free tiers are expensive\n at scale. Source: https://dev.to/robbobobbo/we-tracked-202-developer-tool-pricing-changes-in-2026-heres-whats-really-happening-to-free-tiers-45bk\n- Three viable dev-tool models: free + paid usage/team features, per-project, per-workload.\n Source: https://business.daily.dev/resources/developer-tool-pricing-strategy-free-tier-enterprise-contracts/\n- Usage-based and credLine truncated
|
||||
"pendingEdit": {
|
||||
"old_string": "Effort S-M (human: ~2 weeks",
|
||||
"new_string": "Effort M (human: ~2 weeks",
|
||||
"replace_all": false
|
||||
}
|
||||
}
|
||||
@@ -346,21 +346,28 @@ function currentEditPreview(preview: string, r: any, config: string, cwd: string
|
||||
const firstLine = before.slice(0, at).split(/\r?\n/).length;
|
||||
const oldLast = firstLine + input.old_string.split(/\r?\n/).length - 1;
|
||||
const newLast = firstLine + input.new_string.split(/\r?\n/).length - 1;
|
||||
const rows: Array<{line: number; kind: string; text: string; clipped?: boolean}> = [];
|
||||
const rows: Array<{line: number; kind: string; text: string; clipped?: boolean; gutter?: number}> = [];
|
||||
let leading: {kind: string; text: string} | undefined;
|
||||
for (const line of preview.split('\n')) {
|
||||
if (!line.trim() || /^[╌─━]{3,}[ \t]*$/.test(line)) continue;
|
||||
const numbered = /^ {0,3}([1-9]\d*) ([ +\-])(.*)$/.exec(line);
|
||||
// A trimmed empty unchanged row keeps only its number.
|
||||
const numbered = /^( {0,3}([1-9]\d*) )([ +\-])(.*)$/.exec(line) ?? /^( {0,3}([1-9]\d*))()[ \t]*$/.exec(line);
|
||||
if (numbered) {
|
||||
const kind = numbered[3] || ' ';
|
||||
if (leading) {
|
||||
// A wrapped first row has no coordinate. The next same-kind numbered
|
||||
// row anchors its complete visible suffix to the preceding source line.
|
||||
if (leading.kind !== numbered[2] || Number(numbered[1]) < 2) return false;
|
||||
rows.push({line:Number(numbered[1])-1,...leading,clipped:true}); leading=undefined;
|
||||
if (leading.kind !== kind || Number(numbered[2]) < 2) return false;
|
||||
rows.push({line:Number(numbered[2])-1,...leading,clipped:true}); leading=undefined;
|
||||
}
|
||||
rows.push({line:Number(numbered[1]),kind:numbered[2]!,text:numbered[3]!}); continue;
|
||||
rows.push({line:Number(numbered[2]),kind,text:numbered[4] ?? '',gutter:numbered[1]!.length}); continue;
|
||||
}
|
||||
const wrapped = /^ {4,5}([+\-])(.*)$/.exec(line), last = rows.at(-1);
|
||||
const last = rows.at(-1);
|
||||
// An unchanged row wraps under its own gutter with a blank marker column.
|
||||
if (last?.kind === ' ' && last.gutter && line.startsWith(' '.repeat(last.gutter + 1))) {
|
||||
last.text += line.slice(last.gutter + 1); continue;
|
||||
}
|
||||
const wrapped = /^ {4,5}([+\-])(.*)$/.exec(line);
|
||||
if (!wrapped) return false;
|
||||
if (!last) {
|
||||
if (leading && leading.kind !== wrapped[1]) return false;
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
import {expect,test} from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import {createHash} from 'node:crypto';
|
||||
import {createFilePermissionRecorder,currentFilePermissionEpoch} from './helpers/plan-count-file-permission';
|
||||
import {readPlanCountTranscript} from './helpers/plan-count-transcript';
|
||||
import {createPlanCountPermissionGuard} from './helpers/claude-pty-runner';
|
||||
import captured from './fixtures/plan-floor-edit-wrap-37176835584.json';
|
||||
|
||||
// PR run 37176835584: the floor's owned Edit menu stayed on screen until the
|
||||
// 600 s timeout because unchanged rows soft-wrap under a gutter with a blank
|
||||
// marker column, which the cropped-preview parser refused.
|
||||
function fixture(){
|
||||
const dir=fs.mkdtempSync(path.join(os.tmpdir(),'floor-edit-wrap-')),cwd=path.join(dir,'owned'),config=path.join(dir,'config');fs.mkdirSync(cwd);
|
||||
const expected=path.join(cwd,captured.basename),sid='46bdc2d6-b244-41d6-a97d-2694f1639cb2',id='toolu_floorEditWrap37176835584';
|
||||
const journal=path.join(config,'projects','owned',sid+'.jsonl');fs.mkdirSync(path.dirname(journal),{recursive:true});
|
||||
const recorder=createFilePermissionRecorder(cwd,config,expected)!;
|
||||
const startedAt=Date.now()-60_000,timestamp=new Date(startedAt+1000).toISOString();
|
||||
fs.writeFileSync(recorder.file,JSON.stringify({cwd,expected,sessionId:sid,transcriptPath:journal,seenIds:[`${sid}:${id}`],
|
||||
pendingId:`${sid}:${id}`,completedId:null,completedIds:[],timestamp}));
|
||||
fs.writeFileSync(expected,captured.priorWrite);
|
||||
const block={type:'tool_use',id,name:'Edit',input:{...captured.pendingEdit,file_path:expected}};
|
||||
fs.writeFileSync(journal,JSON.stringify({cwd,sessionId:sid,isSidechain:false,timestamp,message:{role:'assistant',content:[{type:'text',text:'Updating the effort estimate.'},block]}})+'\n');
|
||||
const read=(screen=captured.screen)=>currentFilePermissionEpoch(recorder.file,expected,cwd,config,startedAt,readPlanCountTranscript(config,cwd),screen);
|
||||
return {pendingId:`${sid}:${id}`,expected,read,close(){recorder.dispose();fs.rmSync(dir,{recursive:true,force:true});}};
|
||||
}
|
||||
|
||||
test('the captured floor viewport with wrapped unchanged rows binds the owned Edit and grants once',()=>{
|
||||
expect(createHash('sha256').update(captured.screen).digest('hex')).toBe(captured.screenSha256);
|
||||
expect(captured.screen).toMatch(/^ {6}man: ~4-6 weeks/m);
|
||||
const f=fixture();try{
|
||||
expect(f.read()?.pendingId).toBe(f.pendingId);
|
||||
const guard=createPlanCountPermissionGuard();
|
||||
expect(guard(captured.screen,'',f.read())).toBe('grant');
|
||||
expect(guard(captured.screen,'',f.read())).toBe('handled');
|
||||
}finally{f.close();}
|
||||
});
|
||||
|
||||
test('a trimmed empty unchanged row is still compared with its source line',()=>{
|
||||
const f=fixture();try{
|
||||
expect(f.read(captured.screen.replace(/^ 212 +$/m,' 212'))?.pendingId).toBe(f.pendingId);
|
||||
expect(f.read(captured.screen.replace(/^ 212 +$/m,' 212 stray'))).toBeNull();
|
||||
}finally{f.close();}
|
||||
});
|
||||
|
||||
test.each([
|
||||
['changed wrapped unchanged row',(s:string)=>s.replace('man: ~4-6 weeks','man: ~9 weeks')],
|
||||
['extra wrapped unchanged row',(s:string)=>s.replace(/^( {6}man: [^\n]*\n)/m,'$1 injected text\n')],
|
||||
['wrapped unchanged row after an added row',(s:string)=>s.replace(/\n( 210 )/,'\n stray\n$1')],
|
||||
['four-column unchanged wrap',(s:string)=>s.replace(/^ {6}man:/m,' man:')],
|
||||
['quoted unchanged wrap',(s:string)=>s.replace(/^ {6}man:/m,' > man:')],
|
||||
['changed numbered unchanged row',(s:string)=>s.replace(' 210 C) Smoke-test intent',' 210 C) Smoke-test demand')],
|
||||
['changed added row',(s:string)=>s.replace('+rwise redirect','+rwise ignore')],
|
||||
])('%s is refused',(_,change)=>{
|
||||
const f=fixture();try{
|
||||
const screen=change(captured.screen);expect(screen).not.toBe(captured.screen);
|
||||
expect(f.read(screen)).toBeNull();
|
||||
expect(createPlanCountPermissionGuard()(screen,'',f.read(screen))).not.toBe('grant');
|
||||
}finally{f.close();}
|
||||
});
|
||||
Reference in new issue
Block a user