fix: Close ce-polish-beta detection gaps from PR #568 feedback

Overview

Address four concrete detection/resolution gaps in ce-polish-beta raised by @tmchow on EveryInc/compound-engineering-plugin#568:

1. Framework coverage — Nuxt, SvelteKit, Remix, Astro fall through to unknown (the commenter calls them "table stakes alongside Next and Vite")
2. Monorepo blind spot — detect-project-type.sh only inspects the repo root, so a Turborepo with apps/web/next.config.js returns unknown
3. Package-manager detection is documented in prose but not implemented; Next/Vite stubs silently write npm run dev on pnpm/yarn/bun projects
4. Port cascade is lossy — .env reader doesn't strip quotes or trailing comments, AGENTS.md/CLAUDE.md grep hits unrelated doc references, no probe of next.config.* / vite.config.* / config/puma.rb / docker-compose.yml

All four are detection/resolution bugs in an already-shipped beta skill (disable-model-invocation: true, so no auto-trigger regression risk). Fix scope is the skill's own scripts/ and references/ trees plus the Phase 3 wiring in SKILL.md.

Problem Frame

Polish's dev-server lifecycle (Phase 3 in SKILL.md) has three resolution jobs:

• What project type is this? → scripts/detect-project-type.sh
• How do I start it? → per-type recipe in references/dev-server-<type>.md, substituted into a launch.json stub
• What port will it bind to? → inline cascade documented in references/dev-server-detection.md

All three jobs currently fail for common-but-unhandled shapes (monorepos, Nuxt/Astro, pnpm-only repos, quoted .env values). Users hit these gaps the first time they run polish on anything outside the four project types the skill was bootstrapped with (rails, next, vite, procfile). The fallback — "ask the user to author .claude/launch.json" — works but pushes onto the user a discovery problem the skill should do itself.

Feedback is the first real contact the skill has had with a reviewer outside the original plan, and it lines up with hazards already flagged in references/dev-server-vite.md ("SvelteKit, SolidStart, Qwik City, and Astro all use Vite… Different default ports apply") and references/dev-server-next.md ("Monorepo roots: users should set cwd… to the specific Next app"). The skill knew these were gaps and punted — this plan closes the punt.

Requirements Trace

• R1. Nuxt, SvelteKit, Astro, and Remix are recognized first-class project types (no longer fall through to unknown).
• R2. detect-project-type.sh finds a framework config inside a monorepo workspace (up to a bounded depth) and returns a type + relative cwd, so the stub-writer can populate cwd in launch.json without user intervention.
• R3. Next and Vite stubs use the package manager indicated by the lockfile (pnpm / yarn / bun / npm) instead of hard-coding npm.
• R4. Port resolution prefers authoritative config files (framework config, config/puma.rb, Procfile.dev, docker-compose.yml) over prose references. .env parsing correctly strips surrounding quotes and trailing # comment. The noisy AGENTS.md/CLAUDE.md grep is removed.
• R5. Existing users are not regressed. Repos that previously detected correctly continue to detect the same type; repos with .claude/launch.json are unaffected (launch.json still wins).
• R6. Each new or modified script has unit-test coverage in tests/skills/ mirroring the existing ce-polish-beta-dev-server.test.ts harness (tmp git repo, Bun.spawn, exit-code + stdout assertions).

Scope Boundaries

• Not adding Python (Django, Flask, FastAPI), Go, Elixir/Phoenix, Deno/Fresh, Angular, Gatsby, Expo, Electron, Tauri, Storybook, or Ruby non-Rails (Sinatra, Hanami). Trevor listed these as gaps; they each need their own recipe file and dev-server conventions, and together they would roughly double the skill's surface area. Defer to a follow-up plan.
• Not changing .claude/launch.json priority — launch.json always wins over auto-detect. This plan only improves what auto-detect does when launch.json is absent.
• Not rewriting the IDE handoff, kill-by-port, or reachability probe in Phase 3.5/3.6. Those are unaffected.
• Not changing headless-mode semantics. All new scripts are probes; they don't mutate state, so headless rules ("never write .claude/launch.json, never kill without token") are preserved.
• Not adding a framework config parser beyond a conservative regex. Arbitrary JS/TS config files can set port via computed expressions the regex won't catch; when the probe misses, the cascade falls through to framework defaults. Document this as best-effort, not authoritative.
• Not bumping plugin version, marketplace version, or writing a release entry. Per repo AGENTS.md, release-please owns that.

Context & Research

Relevant Code and Patterns

• plugins/compound-engineering/skills/ce-polish-beta/scripts/detect-project-type.sh — current root-only classifier with precedence rules (rails beats procfile, multiple for real disambiguation)
• plugins/compound-engineering/skills/ce-polish-beta/scripts/read-launch-json.sh — existing script that emits sentinel outputs (__NO_LAUNCH_JSON__, __INVALID_LAUNCH_JSON__, __MISSING_CONFIGURATIONS__, __CONFIG_NOT_FOUND__). The sentinel pattern is the convention new scripts should follow for signaling "no match, fall through"
• plugins/compound-engineering/skills/ce-polish-beta/scripts/parse-checklist.sh — pattern for set-unsafe set -u, bash regex ([[ =~ ]]), and awk/jq composition within a single script. New scripts should match this style (no set -euo pipefail; the existing scripts use set -u only, by convention)
• plugins/compound-engineering/skills/ce-polish-beta/references/dev-server-<rails|next|vite|procfile>.md — per-type recipe shape: Signature, Start command, Port, Stub generation, Common gotchas
• plugins/compound-engineering/skills/ce-polish-beta/references/launch-json-schema.md — stub templates grouped by project type; the stub-writer block to parameterize
• tests/skills/ce-polish-beta-dev-server.test.ts — test harness pattern: tmp git repo, touch signature files, invoke script via Bun.spawn, assert exitCode + stdout.trim(). All new scripts follow this shape.
• plugins/compound-engineering/skills/ce-polish-beta/SKILL.md Phase 3.2 (lines 272-291) — project-type routing table; the surface that needs extending for new types and the <type>@<cwd> return variant
• plugins/compound-engineering/skills/ce-polish-beta/SKILL.md Phase 3.3 (lines 293-303) — stub-writer; where package-manager substitution and cwd population land

Institutional Learnings

None directly applicable; this work extends patterns already proven in the same skill.

Cross-Repo Reference (informational only)

plugins/compound-engineering/skills/test-browser/SKILL.md has an inline port cascade that polish's dev-server-detection.md is a copy of (per the self-contained-skill rule). This plan does not modify test-browser — the two cascades stay independent by design. Note for maintainers: if test-browser adopts a parallel resolve-port script later, the two skills will need the standard manual-sync note updated.

Key Technical Decisions

• Decision: detect-project-type.sh returns <type> at root and <type>@<cwd> for monorepo hits, never just <cwd>. Rationale: keeps the existing single-token protocol intact for the 90% root-detection case; downstream readers split on @ when present. @ is chosen over : because : is reserved for the outer multi-hit separator (see below). Alternative considered: return structured JSON. Rejected because every other script in scripts/ returns plain-text tokens and consumers use case/awk on them, and JSON would force jq onto a detector that today only uses bash builtins.

• Decision: Output grammar is <type> or <type>@<cwd> for single hits, multiple or multiple:<type>@<cwd>,<type>@<cwd>,... for multi-hits. The four concrete shapes are:

  • next (single hit at root)
  • next@apps/web (single hit in monorepo)
  • multiple (multiple signatures at root — existing behavior, unchanged)
  • multiple:next@apps/web,rails@apps/api (multiple hits across monorepo workspaces, always emitted as type@path pairs even when types are the same) Rationale: : is the outer multi-hit delimiter and @ is the inner type-path delimiter, making the grammar unambiguous under naive awk -F: or bash parameter expansion. Document this explicitly in the script header comment so callers cannot misread it.

• Decision: New scripts accept an optional path as a positional argument, not --cwd. Rationale: every existing script in scripts/ uses positional args (parse-checklist.sh <path>, classify-oversized.sh <path> <path>) or derives cwd from git rev-parse --show-toplevel. Flag-parsing would be a new convention. Follow the existing pattern: optional positional path defaults to git rev-parse --show-toplevel.

• Decision: Expected-no-result sentinels exit 0, not 1. Rationale: the existing convention in read-launch-json.sh (header comment on lines 20-21 of that file) reserves non-zero exit for operational failure only (missing jq, no git root). __NO_PACKAGE_JSON__ and similar sentinels exit 0 with the sentinel on stdout; callers pattern-match on stdout, not exit code.

• Decision: No provenance output on stderr. Rationale: stderr across all existing scripts is reserved for ERROR: ... messages only. Provenance ("resolved_from: framework_config") would break that convention. resolve-port.sh emits a single-line integer on stdout, matching the simplicity of existing scripts. If future debugging surfaces real demand for provenance, add a second script or a --verbose mode in a follow-up — not speculatively.

• Decision: Monorepo probe has a depth cap of 3 and walks only if root detection returned unknown. Rationale: depth 3 covers the common layouts (apps/web/next.config.js, packages/frontend/vite.config.ts, services/api/next.config.js). Running unconditionally would slow the common case and risk false positives when the root is a known type with example configs nested elsewhere (fixtures, templates). Depth 3 is a hard cap because deeper nesting usually means the user already needs to author launch.json.

• Decision: Exclude node_modules/, .git/, vendor/, dist/, build/, coverage/, .next/, .nuxt/, .svelte-kit/, .turbo/, tmp/, fixtures/ from the monorepo probe. Rationale: these directories ship config files as fixtures or build output that the user doesn't own. Without exclusion, a Rails app with node_modules/next/.../examples/ would register as Next, and a monorepo with test fixtures would surface false positives.

• Decision: resolve-package-manager.sh returns one token (npm / pnpm / yarn / bun) plus the start command (stdout line 1 and line 2 respectively) so stub-writer substitution is deterministic. Rationale: pnpm dev and bun run dev use different argv shapes. A single-token return would force the consumer to maintain a lookup table; emitting both the binary and the canonical args keeps all PM-specific knowledge in one place (the resolver).

• Decision: resolve-port.sh replaces the inline dev-server-detection.md cascade. Rationale: the cascade lives in skill prose and has silently-buggy shell (unstripped quotes, noisy grep). Lifting it into a tested script with the sentinel-output convention makes the behavior assertable and fixes the bugs at the same site. dev-server-detection.md becomes a thin pointer to the script with the framework-default table retained.

• Decision: Port cascade probes authoritative config files first, .env* second, default last. Rationale: Trevor's core complaint is that the current cascade prefers prose (AGENTS.md) over config (next.config.js, config/puma.rb). Flipping that ordering restores "the code is the source of truth."

• Decision: Drop the AGENTS.md / CLAUDE.md grep entirely. Rationale: users who need to override have the explicit --port / port: CLI token and the .claude/launch.json escape hatch. Grepping instruction files for port numbers catches unrelated mentions ("connects to Stripe on port 8443", "example: localhost:3000") far more often than it captures a real override.

• Decision: Framework config probes use a conservative regex and treat misses as "no pin, fall through". Rationale: parsing arbitrary JS/TS reliably requires a JS runtime, which polish doesn't ship with. A regex that catches port: 3000, port: "3000", and server: { port: 3000 } literals covers the common patterns. Missed ports fall through to framework default — same behavior as today, just with more chances to catch an explicit value along the way.

Open Questions

Resolved During Planning

• Should Remix get a dedicated signature or route through Vite? Resolved: both. Classic Remix ships remix.config.js without Vite; Remix 2.x+ ships vite.config.ts. Classic pattern gets its own signature in the detector so it resolves without ambiguity; new Remix continues to resolve as vite (the existing Vite recipe already documents SvelteKit/Astro/etc. as framework-on-Vite). The remix recipe notes both paths.

• Should the monorepo probe return all matches or just one? Resolved: return one if there's a single match, multiple with <type>@<path> pairs if several. Multiple matches at depth ≤3 is the genuine disambiguation case the existing multiple sentinel was designed for; the new output is multiple:next@apps/web,next@apps/admin so the interactive prompt in Phase 3.2 can list the options.

• Where does SKILL.md document the new <type>@<cwd> format? Resolved: extend the existing Phase 3.2 routing table with a "Paths with @<cwd> suffix" paragraph and update Phase 3.3 to substitute cwd when present. No new top-level section.

• Does the port resolver need to parse docker-compose.yml? Resolved: yes, but lightly — grep for - "<port>:<port>" under a ports: key on the service named web / app / frontend. Full YAML parsing is out of scope; a line-anchored regex catches the common compose shape and misses gracefully on exotic configs.

Deferred to Implementation

• Exact regex for framework config port probes. Start with port:\\s*[0-9]+ and port:\\s*["']?[0-9]+["']?, tighten if tests surface false positives. Unit 4 owns this.
• Whether pnpm dev should be pnpm dev or pnpm run dev. Both work; pick whichever is idiomatic per the current pnpm docs at the time of implementation and pin it in the resolver's lookup table.
• Whether to probe bun.lock ahead of bun.lockb. Bun recently added a text lockfile format (bun.lock)