Add .gitignore seeding to init for ephemeral .impeccable output#314
Add .gitignore seeding to init for ephemeral .impeccable output#314spa5k wants to merge 2 commits into
Conversation
Init now runs ensure-gitignore.mjs to write a marked block to the shared, committed .gitignore so screenshots, live session/preview/cache dirs, hook caches, and per-dev config.local.json never pollute git status across the team. Shared artifacts (config.json, live/config.json, design.json, critique/*.md) stay tracked. Unlike the existing hook/live runtime helpers, which write machine-local .git/info/exclude lazily, this targets .gitignore at init time so every clone is covered up front.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2c101f6. Configure here.
Cursor Bugbot on PR pbakaus#314 flagged two issues. (1) Patterns were root-anchored (/.impeccable/...) so they missed a nested monorepo .impeccable (apps/web/.impeccable/...); dropped the leading slash to match HOOK_LOCAL_IGNORE_PATTERNS / LIVE_IGNORE_PATTERNS. (2) detectTrackedArtifacts used fs.existsSync, reporting untracked/ignored files as committed; replaced with git ls-files based analyzeTracked that returns gitAvailable, tracked (confirmed shared artifacts), and needsUntrack (committed ephemeral files -> git rm --cached candidates). init Step 7 wording updated to match.
Greptile SummaryThis PR seeds the committed
Confidence Score: 3/5Safe to merge after addressing the missing error-handling in
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[node ensure-gitignore.mjs] --> B{--check flag?}
B -- yes --> C[checkImpeccableGitignore]
B -- no --> D[ensureImpeccableGitignore]
C --> E[resolveRepoRoot]
D --> E
E --> F{.git found?}
F -- yes --> G[repo root]
F -- no --> H[fallback: cwd]
D --> J{marker block present?}
J -- yes --> K[replace block in-place]
J -- no --> L[append block with blank separator]
K --> M[writeFileSync if changed]
L --> M
C --> N{.gitignore exists?}
N -- no --> O[present:false stale:false]
N -- yes --> P[match marker regex]
P --> Q[present / stale flags]
D --> R[analyzeTracked via git ls-files]
C --> R
R --> S{git available?}
S -- no --> T[gitAvailable:false empty lists]
S -- yes --> U[classify .impeccable/ files]
U --> V[tracked: shared artifacts
needsUntrack: ephemeral files]
M --> W[JSON output]
O --> W
Q --> W
T --> W
V --> W
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[node ensure-gitignore.mjs] --> B{--check flag?}
B -- yes --> C[checkImpeccableGitignore]
B -- no --> D[ensureImpeccableGitignore]
C --> E[resolveRepoRoot]
D --> E
E --> F{.git found?}
F -- yes --> G[repo root]
F -- no --> H[fallback: cwd]
D --> J{marker block present?}
J -- yes --> K[replace block in-place]
J -- no --> L[append block with blank separator]
K --> M[writeFileSync if changed]
L --> M
C --> N{.gitignore exists?}
N -- no --> O[present:false stale:false]
N -- yes --> P[match marker regex]
P --> Q[present / stale flags]
D --> R[analyzeTracked via git ls-files]
C --> R
R --> S{git available?}
S -- no --> T[gitAvailable:false empty lists]
S -- yes --> U[classify .impeccable/ files]
U --> V[tracked: shared artifacts
needsUntrack: ephemeral files]
M --> W[JSON output]
O --> W
Q --> W
T --> W
V --> W
Reviews (2): Last reviewed commit: "Fix: unanchored patterns + git-aware tra..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| '.impeccable/*.png', | ||
| '.impeccable/live/server.json', | ||
| '.impeccable/live/sessions/', | ||
| '.impeccable/live/previews/', | ||
| '.impeccable/live/annotations/', | ||
| '.impeccable/live/cache/', |
There was a problem hiding this comment.
TRACKED_ARTIFACTS missing .impeccable/critique/ coverage
init.md Step 7 lists .impeccable/critique/*.md as a shared artifact that stays tracked, right alongside config.json, design.json, and live/config.json. TRACKED_ARTIFACTS omits critique files entirely, so the tracked report will never include them even if several critique reports are present and committed. Because critique/ is a glob, you can't use a single existsSync call, but a quick fs.readdirSync check on .impeccable/critique/ filtered to *.md would cover the gap and keep the tracked output consistent with what init.md promises.
| export function checkImpeccableGitignore(cwd = process.cwd()) { | ||
| const repoRoot = resolveRepoRoot(cwd); | ||
| const targetPath = path.join(repoRoot, '.gitignore'); | ||
| if (!fs.existsSync(targetPath)) { | ||
| return { | ||
| ok: true, | ||
| file: path.relative(path.resolve(cwd), targetPath).split(path.sep).join('/') || '.gitignore', | ||
| present: false, | ||
| stale: false, | ||
| patterns: [...GITIGNORE_PATTERNS], | ||
| }; | ||
| } | ||
| const existing = fs.readFileSync(targetPath, 'utf-8'); | ||
| const re = markerRegex(); | ||
| const match = existing.match(re); | ||
| return { | ||
| ok: true, | ||
| file: path.relative(path.resolve(cwd), targetPath).split(path.sep).join('/') || '.gitignore', | ||
| present: !!match, | ||
| stale: !!match && match[0] !== blockText(), | ||
| patterns: [...GITIGNORE_PATTERNS], | ||
| }; | ||
| } |
There was a problem hiding this comment.
checkImpeccableGitignore can throw instead of returning { ok: false }
ensureImpeccableGitignore wraps its entire body in a try/catch that returns { ok: false, error, ... } on any I/O failure. checkImpeccableGitignore has no such guard: if .gitignore exists but is unreadable (e.g. a permissions problem), fs.readFileSync on line 249 throws, the CLI's --check path crashes with an unhandled exception, and the caller receives a Node stack trace instead of the documented JSON output. The asymmetry is especially visible in the CLI block, which spreads both results together — the write path can never crash JSON output, but the check path can.
| ## Step 7: Keep ephemeral Impeccable output out of git | ||
|
|
||
| If the project is a git repo (a `.git` entry exists at or above the project root), seed the shared, committed `.gitignore` so screenshots, runtime state, and per-dev overrides never pollute `git status` or get committed by accident. This is team-wide hygiene: every clone benefits, and it covers artifacts (critique/polish PNGs, live sessions) that the runtime `.git/info/exclude` writers in the hook and live mode do not reach and only add lazily on first run. | ||
|
|
||
| Run `node {{scripts_path}}/ensure-gitignore.mjs`. It is idempotent: it writes one marked block (between `# impeccable-ignore-start` / `# impeccable-ignore-end`) to the repo-root `.gitignore`, preserving everything else in the file. The block ignores only ephemeral and machine-local paths under `.impeccable/` (for example `*.png`, `live/sessions/`, `live/previews/`, `config.local.json`, `hook.cache.json`); shared artifacts stay tracked: | ||
|
|
||
| - `.impeccable/config.json`: unified shared config | ||
| - `.impeccable/live/config.json`: framework wiring (Step 6) | ||
| - `.impeccable/design.json`: shared design spec | ||
| - `.impeccable/critique/*.md`: review reports | ||
|
|
||
| The script also prints two lists derived from `git ls-files`: `tracked` (shared artifacts that are confirmed committed and will stay tracked), and `needsUntrack` (ephemeral files like screenshots or `config.local.json` that were committed earlier and now match the new block). **Adding a `.gitignore` rule does not untrack a file that is already committed.** For each path in `needsUntrack`, offer `git rm --cached <path>` to stop tracking it without deleting the working copy. If `needsUntrack` is empty, just report that the ignore block is in place and nothing needs untracking. This step needs no consent: writing `.gitignore` is harmless, reversible, and the block is clearly marked as owned by Impeccable. | ||
|
|
||
| Skip silently (and say nothing) for non-git projects; there is no `.gitignore` to write. | ||
|
|
||
| ## Step 8: Recommend starting points, then wrap up | ||
|
|
||
| Summarize tersely: | ||
| - Register captured (brand / product) |
There was a problem hiding this comment.
bun run test:skill-behavior not mentioned for init.md changes
AGENTS.md specifies: "For changes to … any Setup-touching reference file (init.md, …), also run bun run test:skill-behavior." The PR description records only bun test tests/docs-integrity.test.js tests/build.test.js tests/ensure-gitignore.test.mjs (25/25). The skill-behavior suite spawns real LLMs against the source SKILL.md and asserts on the tool-call trace; it is the only test that catches an LLM failing to execute the new Step 7 or incorrectly re-numbering downstream steps. Adding a new init step is exactly the scenario that suite is designed to guard.
Context Used: AGENTS.md (source)
| if (re.test(existing)) { | ||
| updated = existing.replace(re, block); |
There was a problem hiding this comment.
markerRegex() has no g flag and String.prototype.replace without a global regex replaces only the first match. If a .gitignore ever ends up with two impeccable-ignore-start … impeccable-ignore-end blocks (e.g., from a manual edit or a merge conflict), the call replaces the first block correctly but silently leaves the second. The test suite only covers the single-stale-block case.
|
Thanks for the PR @spa5k. |
2 participants
What
/impeccable initnow seeds the project's shared, committed.gitignoreso ephemeral.impeccable/output never pollutesgit statusor gets committed by accident.Why
.impeccable/accumulates a lot of junk: critique/polish screenshots,live/*.png, per-sessionlive/{sessions,previews,annotations,cache}/, runtimeserver.json, edit logs,config.local.json,hook.cache.json. The existing runtime helpers (ensureHookGitExcludes,ensureLiveGitIgnores) write to machine-local.git/info/excludelazily and never cover screenshots, so a fresh clone sees all of it untracked. Init is the team-wide entry point, so it now writes one marked block to the committed.gitignoreup front. Shared artifacts (config.json,live/config.json,design.json,critique/*.md) stay tracked.Changes
skill/scripts/ensure-gitignore.mjs(new): idempotent marked-block writer for the repo-root.gitignore, with--checkmode andtrackedreporting.skill/reference/init.md: new Step 7 runs it for git repos; wrap-up renumbered to Step 8.tests/ensure-gitignore.test.mjs(new, 13 tests).Validation
bun run buildgreen;bun test tests/docs-integrity.test.js tests/build.test.js tests/ensure-gitignore.test.mjs— 25/25 pass.Note
Low Risk
Changes are limited to init docs, a new optional
.gitignorewriter, and tests; no runtime auth or app logic is modified.Overview
Init gains a new Step 7 that runs
ensure-gitignore.mjsin git repos to idempotently add a marked# impeccable-ignore-start/# impeccable-ignore-endblock to the repo-root.gitignore. The block ignores ephemeral.impeccable/paths (screenshots, live session/preview/cache dirs, hook caches,config.local.json, etc.) using unanchored patterns so nested monorepos match, while shared files likeconfig.json,live/config.json,design.json, andcritique/*.mdstay trackable.The new script also classifies committed paths via
git ls-filesintotrackedvsneedsUntrack, so init can suggestgit rm --cachedfor previously committed junk. Wrap-up moves to Step 8 (Step 2’s follow-up reference updated accordingly). 13 tests cover idempotent writes, stale-block refresh, monorepo paths, and CLI/--checkbehavior.Reviewed by Cursor Bugbot for commit 894f66a. Bugbot is set up for automated code reviews on this repo. Configure here.