mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-01 19:25:10 +02:00
v1.15.0.0 feat: slim preamble + real-PTY plan-mode E2E harness (#1215)
* chore: add gstack skill routing rules to CLAUDE.md Per routing-injection preamble — once-per-project addition that lets agents auto-invoke the right gstack skill instead of answering generically. * refactor: slim preamble resolvers + sidecar-symlink helper Compress prose across 18 preamble resolvers — Voice, Writing Style, AskUserQuestion Format, Completeness Principle, Confusion Protocol, Context Health, Context Recovery, Continuous Checkpoint, Lake Intro, Proactive Prompt, Routing Injection, Telemetry Prompt, Upgrade Check, Vendoring Deprecation, Writing Style Migration, Brain Sync Block, Completion Status, and Question Tuning. Same semantic contract, ~half the bytes. Restored "Treat the skill file as executable instructions" phrase in the plan-mode info section after diagnosing it as load-bearing. Restored "Effort both-scales" rule in AskUserQuestion format. Bonus: scripts/skill-check.ts gains isRepoRootSymlink() so dev installs that mount the repo root at host/skills/gstack as a runtime sidecar (e.g., codex's .agents/skills/gstack) get skipped instead of double-counted. opus-4-7 model overlay gets a Fan-Out directive — explicit instruction to launch parallel reads/checks before synthesis. Net token impact across all generated SKILL.md files: ~140K tokens removed across 47 outputs. Plan-* skills retain full preamble surface (Brain Sync, Context Recovery, Routing Injection) — load-bearing functionality that early slim attempts incorrectly cut. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: regenerate SKILL.md outputs after preamble slim bun run gen:skill-docs --host all output. Mirrors the resolver changes in the previous commit. 47 generated SKILL.md files plus 3 ship-skill golden fixtures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(test): real-PTY harness for plan-mode E2E tests Adds test/helpers/claude-pty-runner.ts. Spawns the actual claude binary via Bun.spawn({terminal:}) (Bun 1.3.10+ has built-in PTY — no node-pty, no native modules), drives it through stdin/stdout, and parses rendered terminal frames. Pattern adapted from the cc-pty-import branch's terminal-agent.ts but stripped of WS/cookie/Origin scaffolding (not needed for headless tests). Public API: - launchClaudePty(opts) — boots claude with --permission-mode plan|null, auto-handles the workspace-trust dialog, returns a session handle. - session.send / sendKey / waitForAny / waitFor / mark / visibleSince / visibleText / rawOutput / close - runPlanSkillObservation({skillName, inPlanMode, timeoutMs}) — high-level contract for plan-mode skill tests. Returns { outcome, summary, evidence, elapsedMs }. outcome ∈ {asked, plan_ready, silent_write, exited, timeout}. Replaces the SDK-based runPlanModeSkillTest from plan-mode-helpers.ts which never worked. Plan mode renders its native "Ready to execute" confirmation as TTY UI (numbered options with ❯ cursor), not via the AskUserQuestion tool — so the SDK's canUseTool interceptor never fired and the assertion always saw zero questions. Real PTY observes the rendered output directly. Deletes test/helpers/plan-mode-helpers.ts. No production callers remained. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: rewrite 5 plan-mode E2E tests on the real-PTY harness Replaces SDK-based assertions with runPlanSkillObservation contract. Each test launches real claude --permission-mode plan, invokes the skill, and asserts the outcome reaches 'asked' or 'plan_ready' within a 300s budget (no silent Write/Edit, no crash, no timeout). Affected: - test/skill-e2e-plan-ceo-plan-mode.test.ts - test/skill-e2e-plan-eng-plan-mode.test.ts - test/skill-e2e-plan-design-plan-mode.test.ts - test/skill-e2e-plan-devex-plan-mode.test.ts - test/skill-e2e-plan-mode-no-op.test.ts (inPlanMode: false; tests the preamble plan-mode-info no-op path) test/e2e-harness-audit.test.ts — recognize runPlanSkillObservation as a valid coverage path alongside the legacy canUseTool / runPlanModeSkillTest. test/helpers/touchfiles.ts — point the 5 plan-mode test selections and the e2e-harness-audit selection at test/helpers/claude-pty-runner.ts instead of the deleted plan-mode-helpers.ts. Proof: bun test EVALS=1 EVALS_TIER=gate on these 5 files runs sequentially in 790s and passes 5/5. Same tests were 0/5 on origin/main, on v1.0.0.0, and on this branch with the SDK harness. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: align unit tests with slim resolvers + exempt 27MB security fixture - test/skill-validation.test.ts: assert the slim Completeness Principle shape (Completeness: X/10, kind-note language) instead of the old Compression table. Remove the 3 tier-1 skills from the spot-check list (they intentionally don't carry the full Completeness Principle section). Exempt browse/test/fixtures/security-bench-haiku-responses.json (27MB deterministic replay fixture for BrowseSafe-Bench) from the 2MB tracked-file gate. The gate was actually failing on origin/main since the fixture was added in v1.6.4.0 — this is a side-fix to a real regression. - test/brain-sync.test.ts: developer-machine-safe assertion for GSTACK_HOME override (compare config contents before/after instead of asserting the absence of a string that may legitimately exist). - test/gen-skill-docs.test.ts: new tests for the slim — plan-review preambles stay under the post-slim budget (~33KB), Voice + Writing Style sections stay compact, and the slim Voice section preserves the load-bearing semantic contract (lead-with-the-point, name-the-file, user-outcome framing, no-corporate, no-AI-vocab, user-sovereignty). Update path-leakage scan to allow repo-root sidecar symlinks. - test/writing-style-resolver.test.ts: assert the compact contract (gloss-on-first-use, outcome-framing, user-impact, terse-mode override) instead of the old 6-numbered-rules shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v1.13.1.0) Slim preamble work + real-PTY plan-mode E2E harness on top of v1.13.0.0. SKILL.md corpus -25.5% (3.08 MB → 2.30 MB, ~196K tokens). 5 plan-mode tests go from 0/5 to 5/5 (790s sequential), the first time those tests have ever passed. Side-fixes for the 27MB security fixture warning and the sidecar-symlink double-count. Reverts the Fan-Out directive accidentally restored to opus-4-7.md — v1.10.1.0's overlay-efficacy harness measured -60pp fanout vs baseline when the nudge was active. The intentional removal stays. TODOS: - Pre-existing test failures from v1.12.0.0 ship: RESOLVED on main + this branch - security-bench-haiku-responses.json size gate: RESOLVED via warn-only + exemption Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(test): harness primitives — parseNumberedOptions + budget regression utils claude-pty-runner.ts: - parseNumberedOptions(visible) anchors on the latest "❯ 1." cursor and returns {index, label}[]; tests that route on option labels can find indices without hard-coding positions - isPermissionDialogVisible(visible) detects file-grant + workspace-trust + bash-permission shapes (multiple regex variants) - isNumberedOptionListVisible: replaced \b2\. word-boundary regex with [^0-9]2\. — stripAnsi removes TTY cursor-positioning escapes that collapse "Option 2." to "Option2.", and \b fails on word-to-word eval-store.ts: - findBudgetRegressions(comparison, opts?) — pure function returning tests where tools or turns grew >cap× vs prior run; floors at 5 prior tools / 3 prior turns to avoid noise on tiny numbers - assertNoBudgetRegression() — wrapper that throws with full violation list. Env override GSTACK_BUDGET_RATIO helpers-unit.test.ts: 23 unit tests covering empty/sparse/wrap-around buffers for parseNumberedOptions, plus regression-floor + env-override cases for findBudgetRegressions/assertNoBudgetRegression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: register 6 real-PTY E2E touchfiles + UI-heavy plan fixture touchfiles.ts: - 6 new entries in E2E_TOUCHFILES keyed to the new test files - 6 matching E2E_TIERS classifications: 3 gate (auq-format-pty, plan-design-with-ui-scope, budget-regression-pty), 3 periodic (plan-ceo-mode-routing, ship-idempotency-pty, autoplan-chain-pty) - gate ones are cheap/deterministic; periodic ones run weekly touchfiles.test.ts: - update the "skill-specific change selects only that skill" count from 15 → 18 (plan-ceo-review/SKILL.md change now also selects auq-format-pty, plan-ceo-mode-routing, autoplan-chain-pty) test/fixtures/plans/ui-heavy-feature.md: - planted plan with explicit UI scope keywords (pages, components, Tailwind responsive layout, hover/loading/empty states, modal, toast). Used by plan-design-with-ui-scope and autoplan-chain tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(test): 3 gate-tier real-PTY E2E tests skill-e2e-auq-format-compliance.test.ts (~$0.50/run, 90-130s): - Asserts /plan-ceo-review's first AUQ contains all 7 mandated format elements (ELI10, Recommendation, Pros/Cons with ✅/❌, Net, (recommended) label). Catches drift in the shared preamble resolver that previously took weeks to notice. - Auto-grants permission dialogs that fire during preamble side-effects (touch on .feature-prompted markers in fresh user environments). - Verified PASS in 126s. skill-e2e-plan-design-with-ui.test.ts (~$0.80/run, 50-90s): - Counterpart to the existing no-UI early-exit test. When the input plan DOES describe UI changes, /plan-design-review must NOT early-exit and must reach a real skill AUQ. - Sends the slash command without args, then a follow-up message with the UI-heavy plan description (Claude Code rejects unknown trailing args). Asserts evidence does NOT contain "no UI scope". - Verified PASS in 54s. skill-budget-regression.test.ts (free, gate): - Library-only assertion. Reads the most recent eval file, finds the prior same-branch run via findPreviousRun, computes ComparisonResult, asserts no test exceeded 2× tools or turns. - Branch-scoped: skips with reason if the latest eval was produced on a different branch (cross-branch comparison would be noise). - First-run grace (vacuous pass) when no prior data exists. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(test): 3 periodic-tier real-PTY E2E tests skill-e2e-plan-ceo-mode-routing.test.ts (~$3/run, 6-10 min/case): - Verifies AUQ answer routing: HOLD SCOPE → rigor/bulletproof posture language; SCOPE EXPANSION → expansion/10x/dream language. Each case navigates 8-12 prior AUQs (telemetry, proactive, routing, vendoring, brain, office-hours, premise, approach) before hitting Step 0F. - Periodic, not gate: navigation phase too slow for PR-blocking. V2 expansion to 4 modes (SELECTIVE + REDUCTION) when nav is faster. skill-e2e-ship-idempotency.test.ts (~$3/run, 5-10 min): - Builds a real git fixture with VERSION 0.0.2 already bumped, matching package.json, CHANGELOG entry, pushed to a local bare remote. Runs /ship in plan mode and asserts STATE: ALREADY_BUMPED echoes from the Step 12 idempotency check, OR plan_ready terminates without mutation. - Snapshots VERSION + package.json + CHANGELOG entry count + commit count + branch HEAD before/after; fails if any changed. skill-e2e-autoplan-chain.test.ts (~$8/run, 12-18 min): - Asserts /autoplan phases run sequentially: tees timestamps as each "**Phase N complete.**" marker first appears. Phase 1 (CEO) must precede Phase 3 (Eng); Phase 2 (Design) is optional but if it appears, must sit between 1 and 3. - Auto-grants permission dialogs that fire during phase transitions. All three auto-handle permission dialogs (preamble side-effects on fresh user envs without .feature-prompted-* markers). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: spell out AskUserQuestion everywhere instead of AUQ Per user feedback: don't shorten AskUserQuestion to AUQ — the abbreviation reads as cryptic. Apply across all the new code from this branch: - Rename test/skill-e2e-auq-format-compliance.test.ts → test/skill-e2e-ask-user-question-format-compliance.test.ts - Touchfile entry auq-format-pty → ask-user-question-format-pty (touchfiles.ts + matching assertion in touchfiles.test.ts) - Function rename navigateToModeAuq → navigateToModeAskUserQuestion - Variable auqVisible → askUserQuestionVisible - Outcome literal 'real_auq' → 'real_question' - All comments + JSDoc + CHANGELOG entry write AskUserQuestion in full - "AUQs" plural → "AskUserQuestions" No behavior change. 49/49 free tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: harden v1.15.0.0 CHANGELOG entry against hostile readers Per Garry: write the entry assuming a critic will screencap one line and try to use it as ammunition. Reframed the v1.15.0.0 release-summary to lead with new capability (real-PTY harness, 11 plan-mode tests, +6 new) instead of fix-of-prior- flaw narrative. Removed phrases that critics could weaponize: - "0/5 → 5/5 passing", "finally pass", "∞ (never green)" — drop - "Skill prompts get a 25% haircut" — implied self-inflicted bloat - "770K → 574K tokens" — absolute number lets critics quote "still 574K of bloat"; replaced with relative "−196K tokens per invocation" - "5 plan-mode E2E tests turned out to have never actually passed" — literal admission of long-term breakage; cut entirely - Itemized "Fixed: tests finally pass" entry — moved to Changed with neutral "rewritten on the new harness" framing - "Removed: harness with the runPlanModeSkillTest API that never worked" — replaced with "superseded by claude-pty-runner.ts" Added concrete code receipts to pre-empt "it's just markdown": - Net branch size: −11,609 lines (89 files, +7,240 / −18,849) - 654 lines of TypeScript in test/helpers/claude-pty-runner.ts - 8 new test files, ~1,453 lines of new TS code - 23 helper unit tests + 6 new gate/periodic E2E tests The deletion-heavy net diff (−11.6K lines) is itself the strongest defense against the "bloat" critique — surfaced explicitly in the numbers table. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -54,39 +54,6 @@ scope of that PR; deliberately deferred to keep PTY-import small.
|
||||
|
||||
## Testing
|
||||
|
||||
### Pre-existing test failures surfaced during v1.12.0.0 ship
|
||||
|
||||
**What:** Two remaining test failures on bare main that have been shipping as-is for multiple versions. (The bearer-json secret-scan regression flagged here originally was a real leak path and has been fixed in this PR — see Completed section below.)
|
||||
|
||||
1. `gstack-config gbrain keys > GSTACK_HOME overrides real config dir` (`test/brain-sync.test.ts:104`) — the GSTACK_HOME env override leaks into the real `~/.gstack/config.yaml`. Test asserts real config does NOT contain `gbrain_sync_mode: full` but it does. Either the test environment isn't isolated correctly or `bin/gstack-config` is writing to both locations.
|
||||
2. `Opus 4.7 overlay — pacing directive > keeps Fan out / Effort-match / Literal interpretation nudges` (`test/model-overlay-opus-4-7.test.ts:87`) — v1.10.1.0 (#1166) removed the "Fan out explicitly" nudge from the overlay but the assertion was never updated. Either the nudge should come back (intentional removal reverted) or the test should be updated to match the new expected content.
|
||||
|
||||
**Why:** Both have been green-washing through recent `/ship` runs via "pre-existing test failures skipped: <name>." #1 signals a real config isolation bug; #2 is a stale assertion since the overlay intentionally removed that nudge.
|
||||
|
||||
**Priority:** P0 (both)
|
||||
|
||||
**Effort:** S each. #1 likely a test harness fix in `test/brain-sync.test.ts`'s setup hook. #2 is a one-line test update OR a revert of #1166.
|
||||
|
||||
---
|
||||
|
||||
### `security-bench-haiku-responses.json` is 27MB, violates the 2MB tracked-file gate
|
||||
|
||||
**What:** `browse/test/fixtures/security-bench-haiku-responses.json` landed on main at v1.6.4.0 (PR #1135) at 27MB. The `no compiled binaries in git > git tracks no files larger than 2MB` gate in `test/skill-validation.test.ts:1623` fails on main and on every feature branch that merges main afterward.
|
||||
|
||||
**Why:** The fixture is a legitimate CI replay corpus (real Haiku responses from the 500-case BrowseSafe-Bench) used to verify the ensemble classifier deterministically. But 13x over the 2MB limit means it will keep failing the validation test for every future ship.
|
||||
|
||||
**Pros:** Removes a pre-existing failure that wastes a triage slot in every /ship run.
|
||||
|
||||
**Cons:** Moving to git-lfs adds a dependency. Splitting into chunks risks breaking the bench test. External hosting adds a CI fetch step.
|
||||
|
||||
**Context:** Noticed during workspace-aware-ship /ship on 2026-04-23 when the post-merge test suite flagged this single failure. Introduced on main in PR #1135 (`v1.6.4.0: cut Haiku classifier FP from 44% to 23%`), commit d75402bb. Two reasonable paths: (a) split into multiple ≤2MB chunks and load them in the bench test, (b) move to git-lfs.
|
||||
|
||||
**Effort:** M (human: ~2-3h / CC: ~20 min)
|
||||
**Priority:** P1 (not blocking ship, but every future /ship triages the same failure)
|
||||
**Depends on:** nothing
|
||||
|
||||
---
|
||||
|
||||
## P1: Structural STOP-Ask forcing function across all skills
|
||||
|
||||
**What:** Design and implement a structural forcing function that catches when a skill mandates per-issue AskUserQuestion but the model silently substitutes batch-synthesis. Candidate mechanisms: question-count assertion (skill declares expected question count in frontmatter; post-run audit logs if model fired <N), typed question templates (skill hands the model pre-built AskUserQuestion payloads rather than prose instructions), or a canUseTool-based post-run audit that compares declared-gates-fired vs expected.
|
||||
@@ -1355,6 +1322,36 @@ Shipped in v0.6.5. TemplateContext in gen-skill-docs.ts bakes skill name into pr
|
||||
|
||||
## Completed
|
||||
|
||||
### Slim preamble + real-PTY plan-mode E2E harness (v1.13.1.0)
|
||||
|
||||
- Compressed 18 preamble resolvers; total `SKILL.md` corpus dropped from 3.08 MB to 2.30 MB across 47 outputs (-25.5%, ~196K tokens saved).
|
||||
- Built `test/helpers/claude-pty-runner.ts` — real-PTY harness using `Bun.spawn({terminal:})` (Bun 1.3.10+ has built-in PTY, no `node-pty` needed).
|
||||
- Rewrote 5 plan-mode E2E tests (`plan-ceo`, `plan-eng`, `plan-design`, `plan-devex`, `plan-mode-no-op`); all 5 pass for the first time ever (790s sequential).
|
||||
- Same tests were 0/5 on `origin/main`, on v1.0.0.0, and on this branch with the SDK harness — the SDK couldn't observe Claude's plan-mode confirmation UI.
|
||||
- Side fixes folded in: `scripts/skill-check.ts` sidecar-symlink helper, `test/skill-validation.test.ts` exemption for `browse/test/fixtures/security-bench-haiku-responses.json` (resolves the size-warning noise from main's warn-only conversion).
|
||||
|
||||
**Completed:** v1.13.1.0 (2026-04-25)
|
||||
|
||||
---
|
||||
|
||||
### Pre-existing test failures surfaced during v1.12.0.0 ship — RESOLVED
|
||||
|
||||
- `test/brain-sync.test.ts` GSTACK_HOME isolation fixed on main in v1.13.0.0.
|
||||
- `test/model-overlay-opus-4-7.test.ts` updated on main to match the new overlay content (the v1.10.1.0 removal of "Fan out explicitly" was correct — measured −60pp fanout vs baseline).
|
||||
|
||||
**Completed:** v1.13.0.0 (2026-04-25, on main)
|
||||
|
||||
---
|
||||
|
||||
### `security-bench-haiku-responses.json` size gate — RESOLVED
|
||||
|
||||
- Main converted the 2 MB tracked-file gate to warn-only in v1.13.0.0.
|
||||
- v1.13.1.0 added a `knownLargeFixtures` exemption to suppress the warning for this specific intentional fixture.
|
||||
|
||||
**Completed:** v1.13.1.0 (2026-04-25)
|
||||
|
||||
---
|
||||
|
||||
### Bearer-token secret-scan regression fixed + E2E coverage added for privacy gate + gh auto-create (v1.12.0.0)
|
||||
|
||||
- **Fixed the `bearer-token-json` regression in `bin/gstack-brain-sync`** — the value charset `[A-Za-z0-9_./+=-]{16,}` didn't permit spaces, so auth headers with the standard `Bearer <token>` form (literal space after the scheme name) slipped past the scanner. Added an optional `(Bearer |Basic |Token )?` prefix to the pattern. Validated against 5 positive cases (including the regression fixture) + 3 negative cases (short tokens, non-secret keys, random JSON). The 7-pattern secret scanner now passes all fixtures including bearer-json.
|
||||
|
||||
Reference in New Issue
Block a user