Skip to content

feat: overhaul impeccable skill and refresh agent guidance - #1125

Merged
lavaman131 merged 1 commit into
mainfrom
feat/impeccable-overhaul-and-agent-guidance
May 30, 2026
Merged

feat: overhaul impeccable skill and refresh agent guidance#1125
lavaman131 merged 1 commit into
mainfrom
feat/impeccable-overhaul-and-agent-guidance

Conversation

@lavaman131

@lavaman131 lavaman131 commented May 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

Refreshes agent guidance across the core system prompt, builtin subagents, and workflow skills, and substantially rebuilds the `impeccable` design skill. The connective thread across the agent changes is the "doors"/entrypoints design lens — code and specs are read as sets of entrypoints that carry domain intent, with interior mechanism refined freely while public contracts stay honest. The `impeccable` overhaul adds context-aware command routing, a new design-antipattern detector, rebuilt live-mode browser tooling, and migrates paths/commands to the `.agents/` layout.

Changes

`packages/coding-agent`

  • Replace the static `<engineering_principles>` block in `system-prompt.ts` with dynamic `workflow` tool guidance (discover / run / inspect / steer), gated on whether the `workflow` tool is excluded — keeps the system prompt lean when workflows aren't available.
  • Add 11 test cases in `system-prompt.test.ts` covering the new workflow guidance gate.

`packages/subagents`

  • `code-simplifier`: Rebuilt around the "doors" lens — a program is a set of entrypoints where domain intent lives. Interior mechanism is rewritten freely; public contracts are never silently changed, only flagged as deferred suggestions. Adds a five-principle rubric (name-a-joint, compress-honestly, intent-in-refusals, stranger-across-time, keep-dangerous-doors-few) and explicit interior-vs-boundary decision guidance.
  • `debugger`: Bumped thinking level from `high` → `xhigh`; added `tmux` skill for terminal/TUI debugging.

`packages/workflows` — `create-spec`

  • Rebuild spec philosophy around entrypoints ("doors"): the door set as first-class artifact, five design principles, and a per-entrypoint rubric.
  • Fix `AskUserQuestion` tool reference → `ask_user_question` (lowercase, correct tool name).

`packages/workflows` — `impeccable`

Migration / renames (breaking for local installs):

  • Script paths migrated from `.claude/skills/` → `.agents/skills/`.
  • Skill invocation changed from `/impeccable` → `$impeccable`.
  • `teach` command renamed to `init`; `teach` kept as a deprecated alias.

Pruned reference docs (stale / superseded):
`cognitive-load`, `color-and-contrast`, `heuristics-scoring`, `motion-design`, `personas`, `responsive-design`, `spatial-design`, `teach`, `typography`, `ux-writing`.

New capabilities:

  • `scripts/context.mjs` + `scripts/context-signals.mjs` — per-session project context loader; surfaces `PRODUCT.md` / `DESIGN.md` or signals `NO_PRODUCT_MD` and blocks until `init` is run.
  • `scripts/detect.mjs` + `scripts/detector/` — multi-engine design-antipattern detector with browser-injected, regex, static-HTML (CSS cascade), and visual (screenshot contrast) engines; a rule registry, profiler, and findings formatter.
  • `scripts/palette.mjs` — brand seed color + OKLCH palette generator for greenfield projects.
  • `agents/` directory — `impeccable_asset_producer` and `impeccable_manual_edit_applier` subagent definitions for context-aware command routing.

Rebuilt live-mode tooling:

  • `live-manual-edits-buffer.mjs` / `live-manual-edit-evidence.mjs` / `live-commit-manual-edits.mjs` / `live-discard-manual-edits.mjs` — manual-edit capture, evidence collection, and commit/discard flow.
  • `live-event-validation.mjs` — validates browser-side events before applying.
  • `live-insert-ui.mjs` / `live-insert.mjs` — UI element insertion helpers.
  • `live-copy-edit-agent.mjs` — copy-edit subagent for live text refinement.
  • Substantially expanded `live-browser.js` and `live-server.mjs`.

Refreshed `SKILL.md`: Tightened setup steps (context script first, then command reference, then register reference, then palette for greenfield); updated design laws.

Migration notes

If you have `impeccable` installed locally, update any path references from `.claude/skills/impeccable/` to `.agents/skills/impeccable/` and replace `/impeccable` invocations with `$impeccable`. The `teach` command still works but is deprecated — use `init` going forward.

Test results

  • `bun test packages/coding-agent/test/system-prompt.test.ts` — 11 tests pass.
  • Pre-commit and pre-push hooks (lint + `test:unit`) passed.

Refresh agent-facing guidance across the system prompt, subagents, and
workflow skills, and rebuild the impeccable design skill.

coding-agent:
- Replace the static engineering-principles block in the system prompt
  with workflow-tool guidance (discover/run/inspect/steer workflows),
  gated on whether the `workflow` tool is excluded. Add coverage in
  system-prompt.test.ts.

subagents:
- Rework code-simplifier around the "doors" lens: read code as a set of
  entrypoints that carry domain intent, refine interior mechanism freely
  while keeping public contracts honest, and run a per-entrypoint rubric.
- Bump debugger to xhigh thinking and add the tmux skill for terminal/TUI
  debugging.

workflows/create-spec:
- Rebuild the spec philosophy around entrypoints ("doors"): the door set
  as a first-class artifact, the five principles, and a per-entrypoint
  rubric. Fix the AskUserQuestion reference to ask_user_question.

workflows/impeccable:
- Migrate script paths from .claude/skills to .agents/skills and switch
  invocations from /impeccable to $impeccable.
- Rename the teach command to init (with a deprecated teach alias) and
  prune deprecated reference docs (cognitive-load, color-and-contrast,
  motion-design, personas, responsive-design, spatial-design, teach,
  typography, ux-writing, heuristics-scoring).
- Add context/context-signals/detect/palette scripts and a bundled
  detector, plus an agents/ directory, for context-aware command routing.
- Rebuild live-mode browser/server tooling (manual-edit buffering, event
  validation, UI insertion, copy-edit agent) and refresh SKILL.md design
  laws.

Assistant-model: Claude Opus 4.8
@lavaman131
lavaman131 merged commit 01b1de2 into main May 30, 2026
9 of 10 checks passed
@lavaman131
lavaman131 deleted the feat/impeccable-overhaul-and-agent-guidance branch May 30, 2026 05:07
@claude

claude Bot commented May 30, 2026

Copy link
Copy Markdown

PR review — overhaul impeccable skill and refresh agent guidance

Big PR (93 files, ~25k+/2.8k−). I read system-prompt.ts, the new live-server.mjs, live-event-validation.mjs, live-manual-edits-buffer.mjs, live-copy-edit-agent.mjs, context.mjs, impeccable-paths.mjs, the touched subagent prompts, and skimmed the detector registry. Notes below — most are minor; a handful are worth fixing before merge.

Correctness / bugs

  1. /source path containment is unsound (live-server.mjs:1413). if (!absPath.startsWith(process.cwd())) — with cwd /foo/bar, the path /foo/barbar/leak.html passes this check. Use the same pattern this file already uses at lines 200 and 834: derive rel = path.relative(process.cwd(), absPath) and reject when !rel || rel.startsWith('..') || path.isAbsolute(rel). Worth a fs.realpathSync before the check too — fs.readFileSync will happily follow a symlink inside the repo that points anywhere on disk.

  2. /source always responds text/html (line 1417). Combined with Access-Control-Allow-Origin: *, any HTML/SVG/JSON in the project is served as executable HTML cross-origin. For a localhost dev tool the blast radius is small, but consider sniffing by extension or whitelisting paths the live mode actually needs.

  3. Unbounded request bodies on JSON POST endpoints/manual-edit-stash (1462), /events POST (1789), /poll POST (1909), /manual-edit-repair-decision (1695). Each does body += c with no max size; a misbehaving client (or CSRF/CORS misuse) can exhaust memory. The /annotation handler at line 1280 already shows the right pattern with MAX_ANNOTATION_BYTES — reuse it.

  4. Non-atomic buffer writes (live-manual-edits-buffer.mjs:53). fs.writeFileSync on a JSON file that drives pending-edit state will tear on crash/SIGKILL. The same repo (live-server.mjs:744) correctly uses writeFileSync(tmp); renameSync(tmp, final) for the apply-transaction sidecar — apply that here, since this buffer is similarly load-bearing across recoveries.

  5. Timing-unsafe token comparison. Every endpoint does if (token !== state.token). UUIDv4 + localhost-only makes this very low risk, but the module already imports node:crypto for randomUUID — a timingSafeEqual (length-check first) is essentially free. The check is also duplicated 13+ times: factor a requireAuth(req, res, url) helper.

  6. existingRecord.info.pid is trusted blindly (live-server.mjs:2150). If the JSON on disk lacks pid (corrupted file, older schema), process.kill(undefined, 0) throws a TypeError that surfaces in the catch path as an unrelated message. Validate the shape before using it.

  7. Tiny phrasing bug in system-prompt.ts:203: "You are an expert coding assistant operating named Atomic" — likely "operating as Atomic" or just "named Atomic". The current sentence parses awkwardly.

Behavior consistency

  • Workflow / subagent guidance gating (system-prompt.ts:183-201). These guidance blocks are gated on excludedTools.has(...) only, not on whether the tool is in selectedTools. A caller passing selectedTools: ["read"] (without "workflow") still gets the full workflow block. That is what the new test asserts (workflow guidance suite), so it is intentional — but it diverges from the visibleTools filter applied to the tools listing and may confuse future readers. A one-line comment explaining "guidance is omitted only when the caller explicitly excludes the tool" would help.

  • debugger.md:16 is missing the em-dash separator that the other bullets use: - `tmux` load the tmux skill… should be - `tmux` — load the tmux skill….

Test coverage

This is the biggest gap. The PR ships ~12k lines of new mjs scripts and adds tests only for system-prompt.ts. Several modules have a clearly unit-testable surface and even self-document the intent:

  • live-event-validation.mjs literally says "Extracted for unit testing (insert mode rules)" — but ships untested.
  • live-manual-edits-buffer.mjs: merge semantics on (pageUrl, ref) are the kind of thing tests catch regressions in.
  • context.mjs: extractRegister, compareSemver, the FALLBACK_DIRS resolution order.
  • live-server.mjs#splitManualApplyBatch and friends — chunking is dense logic with many off-by-one risks.
  • live-copy-edit-agent.mjs#parseCopyEditAgentResult — JSON-with-prose extractor that needs golden-file tests.

The whole packages/workflows package has zero tests today. Adding even a thin test/ directory and a couple of bun:test files for the highest-risk helpers would catch regressions cheaply.

Also: CLAUDE.md says tests should use bun:test + node:assert/strict, but system-prompt.test.ts uses vitest (to match the rest of packages/coding-agent/test/). Either migrate packages/coding-agent/test/ to bun:test over time, or update CLAUDE.md to acknowledge that coding-agent is the upstream-pi exception.

Security (defense in depth)

  • runManualEditValidationScript runs whatever is in package.json.scripts['impeccable:manual-edit-validate'] with shell: true. Acceptable for a tool that already writes source — but make this trust boundary explicit in the user-facing docs ("impeccable will execute your impeccable:manual-edit-validate script in a shell").
  • The AI runners (codex --dangerously-bypass-approvals-and-sandbox, claude --permission-mode bypassPermissions) edit source without confirmation. Worth a visible README note so users know the script is allowed to commit changes.

Code structure

  • live-server.mjs is 2,191 lines. Routing, manual-edit transactions, evidence sidecars, SSE broadcast, chunking, lifecycle all live in one file. The manual-edit-apply machinery (lines ~99–870) is the most obvious extraction candidate and is already self-contained behind a few entry points. Even moving it to live-manual-edit-apply.mjs would make the HTTP file much easier to skim.
  • pendingEvents is a linear-scanned array (acknowledgePendingEvent, findAvailablePendingEvent, cancelPendingManualApplyEvents all O(n)). Probably fine in practice, but a Map<id, entry> would tighten the cancel/ack paths if queues ever grow.

Things I liked

  • Schema versioning + tmp-rename pattern on manualApplyTransactionPath and the SSE connected → events event flow are solid.
  • live-event-validation.mjs cleanly enumerates the allowed event shapes and explicitly forbids markup characters in manual-edit newText — good defense against XSS-through-Save.
  • context.mjs update-check throttling (daily poll + weekly re-notify + opt-out env) is well thought through.
  • The "doors" rewrite of code-simplifier and create-spec is a meaningful conceptual improvement; the interior-vs-public-boundary split gives the agent a concrete decision rule rather than vibes.

Suggested priorities before merge

  1. Fix the /source startsWith containment check (add agent instructions #1).
  2. Bound request bodies on the four POST handlers (update readme and mcp servers #3).
  3. Atomic write for the manual-edits buffer (Flora131/feat/add skills #4).
  4. Add at least a smoke test file for live-event-validation.mjs and live-manual-edits-buffer.mjs.
  5. Fix the system-prompt.ts:203 phrasing typo.

Everything else can be follow-up. Nice work on the scope of this refresh.

lavaman131 added a commit that referenced this pull request Jun 29, 2026
Refresh agent-facing guidance across the system prompt, subagents, and
workflow skills, and rebuild the impeccable design skill.

coding-agent:
- Replace the static engineering-principles block in the system prompt
  with workflow-tool guidance (discover/run/inspect/steer workflows),
  gated on whether the `workflow` tool is excluded. Add coverage in
  system-prompt.test.ts.

subagents:
- Rework code-simplifier around the "doors" lens: read code as a set of
  entrypoints that carry domain intent, refine interior mechanism freely
  while keeping public contracts honest, and run a per-entrypoint rubric.
- Bump debugger to xhigh thinking and add the tmux skill for terminal/TUI
  debugging.

workflows/create-spec:
- Rebuild the spec philosophy around entrypoints ("doors"): the door set
  as a first-class artifact, the five principles, and a per-entrypoint
  rubric. Fix the AskUserQuestion reference to ask_user_question.

workflows/impeccable:
- Migrate script paths from .claude/skills to .agents/skills and switch
  invocations from /impeccable to $impeccable.
- Rename the teach command to init (with a deprecated teach alias) and
  prune deprecated reference docs (cognitive-load, color-and-contrast,
  motion-design, personas, responsive-design, spatial-design, teach,
  typography, ux-writing, heuristics-scoring).
- Add context/context-signals/detect/palette scripts and a bundled
  detector, plus an agents/ directory, for context-aware command routing.
- Rebuild live-mode browser/server tooling (manual-edit buffering, event
  validation, UI insertion, copy-edit agent) and refresh SKILL.md design
  laws.

Assistant-model: Claude Opus 4.8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant