Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions codev/plans/882-refactor-extract-gitignore-hel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# PIR Plan: Extract gitignore helpers out of `scaffold.ts`

## Understanding

`packages/codev/src/lib/scaffold.ts` was extracted in Maintenance Run 0004 to deduplicate logic shared between `codev init` and `codev adopt`. Its header still claims to be "Scaffold utilities for codev init and adopt commands", but the file now also contains:

- `updateGitignore()` — used by `adopt` to merge a Codev block into an existing `.gitignore` (line-level backfill of missing entries).
- `backfillGitignore()` — added in #881 for `codev update`, repairing stale gitignore state in long-lived projects.
- `parseEntryLines()` — internal helper used by `backfillGitignore`.
- The three gitignore-shaped types (`BackfillGitignoreResult`, `BackfillGitignoreOptions`, `UpdateGitignoreResult`).

The grep audit confirms the move list is exhaustive — the only external readers of these symbols are `init.ts`, `adopt.ts`, `update.ts`, and `__tests__/scaffold.test.ts` (verified via `grep -rn` on `packages/codev/src` and `packages/codev/tests`).

## Proposed Change

Pure file move + import rewire. No behavior changes, no function renames, no further pruning of `scaffold.ts`. After this PR:

- `packages/codev/src/lib/gitignore.ts` owns everything gitignore-related.
- `scaffold.ts` is back to true scaffolding helpers — directory creation, skeleton copying, root-file templating.
- Three command files and the test split point to the new module.

## Files to Change

### New file

- `packages/codev/src/lib/gitignore.ts` — new module. Contains:
- `CODEV_GITIGNORE_ENTRIES` (const, exported)
- `FULL_GITIGNORE_CONTENT` (const, exported)
- `createGitignore()` (exported)
- `updateGitignore()` (exported)
- `backfillGitignore()` (exported)
- `parseEntryLines()` (internal, not exported — same visibility as today)
- Types: `UpdateGitignoreResult` (currently interface, kept as-is), `BackfillGitignoreOptions` (interface), `BackfillGitignoreResult` (currently `export interface` — preserved as exported)
- A short module header explaining "gitignore management for init / adopt / update"
- `import * as fs from 'node:fs'; import * as path from 'node:path';`

### Modified files

- `packages/codev/src/lib/scaffold.ts:9-34` — remove `CODEV_GITIGNORE_ENTRIES` and `FULL_GITIGNORE_CONTENT` constants.
- `packages/codev/src/lib/scaffold.ts:222-331` — remove `createGitignore`, `UpdateGitignoreResult` interface, `updateGitignore`, `BackfillGitignoreOptions` interface, `BackfillGitignoreResult` interface, `parseEntryLines`, `backfillGitignore`.
- `packages/codev/src/lib/scaffold.ts:1-4` — update header to drop the "scaffold" gloss only enough to stay accurate (it's still used by init / adopt / update for directory creation and skeleton copying); no further pruning.
- `packages/codev/src/commands/init.ts:13-19` — split import: keep `createUserDirs, createProjectsDir, copySkills, copyRootFiles` from `../lib/scaffold.js`; add `import { createGitignore } from '../lib/gitignore.js';`.
- `packages/codev/src/commands/adopt.ts:14-20` — same split: keep scaffold imports; add `import { updateGitignore } from '../lib/gitignore.js';`.
- `packages/codev/src/commands/update.ts:25-30` — same split: keep `copySkills, copyRootFiles` from scaffold; add `import { backfillGitignore, CODEV_GITIGNORE_ENTRIES } from '../lib/gitignore.js';`.

### Test split

- `packages/codev/src/__tests__/gitignore.test.ts` — **new file**. Moves these `describe` blocks verbatim out of `scaffold.test.ts`:
- `describe('createGitignore', ...)` (current scaffold.test.ts:277–300)
- `describe('updateGitignore', ...)` (302–361)
- `describe('CODEV_GITIGNORE_ENTRIES', ...)` (363–374)
- `describe('backfillGitignore (issue #880)', ...)` (376–480)
- Wraps them in a fresh `describe('Gitignore Utilities', ...)` outer block with its own minimal `beforeEach` / `afterEach` (just `tempDir` — these tests don't need `mockSkeletonDir` or the skeleton fixtures the scaffold tests build).
- Imports come from `../lib/gitignore.js`.
- `packages/codev/src/__tests__/scaffold.test.ts` — remove the four moved `describe` blocks (lines 277–480) and the four corresponding import names from line 10–20. The scaffold-only fixtures (skeleton templates, consult-types, roles directories) stay. Keep the `describe('projectlist removal (Spec 0126)', ...)` regression block — it reads `scaffold.ts` source directly and remains correct.
- Optional regression addition (one new test in `scaffold.test.ts`) verifying `scaffold.ts` source no longer contains `gitignore` / `CODEV_GITIGNORE_ENTRIES` / `backfillGitignore` strings, mirroring the existing `projectlist` regression pattern. Cheap insurance against accidental re-introduction. **Inclusion decision deferred to dev-approval review.**

## Risks & Alternatives Considered

- **Risk: missed import site.** Mitigation: the grep audit covered both `packages/codev/src` and `packages/codev/tests` and found exactly the four files listed above. Will re-run grep after the edits to confirm zero stragglers, plus `pnpm --filter @cluesmith/codev build` (tsc) catches any missed symbol.
- **Risk: changing test names changes CI selectors.** Mitigation: outer describe label is the only thing changing (`'Scaffold Utilities'` → `'Gitignore Utilities'`). Inner test names and assertions are untouched. We don't filter CI by describe-name anywhere I could find.
- **Risk: `parseEntryLines` visibility regression.** Today it's module-private (not exported). Keeping it private in `gitignore.ts` preserves exactly that — no test reaches it directly, only via `backfillGitignore`.
- **Alternative: re-export the moved symbols from `scaffold.ts` for back-compat.** Rejected — internal module, no external consumers, three call sites all updated in this PR. Re-exports would just leave a dead breadcrumb.
- **Alternative: also pull `parseEntryLines` out as a separately exported helper.** Rejected — issue explicitly says "no renaming of the functions themselves" and "one refactor at a time." `parseEntryLines` stays internal.
- **Alternative: bundle a `copyRoles` → `roles.ts` move in the same PR.** Rejected per the issue's "Out of scope" — one refactor at a time.

## Test Plan

This is a pure move with no behavior change. Verification is mostly mechanical:

- **Static**:
- `pnpm --filter @cluesmith/codev build` — must be clean (tsc catches any missed import).
- `grep -rn -E '(createGitignore|updateGitignore|backfillGitignore|CODEV_GITIGNORE_ENTRIES|FULL_GITIGNORE_CONTENT|parseEntryLines|BackfillGitignoreResult|BackfillGitignoreOptions|UpdateGitignoreResult)' packages/codev/src packages/codev/tests` — expect references only in the new `gitignore.ts`, the three command files, and `gitignore.test.ts`. Zero hits in `scaffold.ts` or `scaffold.test.ts`.
- **Unit tests**:
- `pnpm --filter @cluesmith/codev test -- gitignore` — the moved suite passes in its new home.
- `pnpm --filter @cluesmith/codev test -- scaffold` — remaining scaffold tests still green; `projectlist removal` regression still passes.
- `pnpm --filter @cluesmith/codev test` — full suite green. Existing `init.test.ts` / `adopt.test.ts` / `update.test.ts` exercise the rewired imports end-to-end (init creates a fresh gitignore, adopt merges into existing, update backfills missing entries).
- **Manual smoke** (only if any test fails):
- `node packages/codev/dist/cli.js init /tmp/codev-init-smoke --yes` → `.gitignore` exists with the expected entries.
- `cd /tmp/existing && echo 'node_modules/' > .gitignore && node /path/to/codev/dist/cli.js adopt --yes` → entries merged, `node_modules/` preserved.

No cross-platform / device testing needed — this is internal Node-only code with no UI surface.
30 changes: 30 additions & 0 deletions codev/projects/882-refactor-extract-gitignore-hel/status.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
id: '882'
title: refactor-extract-gitignore-hel
protocol: pir
phase: verified
plan_phases: []
current_plan_phase: null
gates:
plan-approval:
status: approved
requested_at: '2026-05-27T11:12:10.553Z'
approved_at: '2026-05-27T11:12:21.788Z'
dev-approval:
status: approved
requested_at: '2026-05-27T11:19:54.392Z'
approved_at: '2026-05-27T11:22:06.722Z'
pr:
status: approved
requested_at: '2026-05-27T11:26:17.341Z'
approved_at: '2026-05-27T11:29:19.402Z'
iteration: 1
build_complete: false
history: []
started_at: '2026-05-27T11:09:08.755Z'
updated_at: '2026-05-27T11:29:30.734Z'
pr_history:
- phase: review
pr_number: 884
branch: builder/pir-882
created_at: '2026-05-27T11:23:38.092Z'
pr_ready_for_human: false
61 changes: 61 additions & 0 deletions codev/reviews/882-refactor-extract-gitignore-hel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# PIR Review: Extract gitignore helpers out of `scaffold.ts`

Fixes #882

## Summary

Moved the three gitignore management helpers (`createGitignore`, `updateGitignore`, `backfillGitignore`), their constants (`CODEV_GITIGNORE_ENTRIES`, `FULL_GITIGNORE_CONTENT`), the private `parseEntryLines` helper, and the three gitignore-shaped types out of `packages/codev/src/lib/scaffold.ts` into a new dedicated `packages/codev/src/lib/gitignore.ts`. `scaffold.ts` now holds only genuine scaffolding helpers (directory creation, skeleton copying, root-file templating). Pure file move + import rewire — zero behavior change, no renames, no function-shape changes.

## Files Changed

- `packages/codev/src/lib/gitignore.ts` (+151 / -0, new)
- `packages/codev/src/lib/scaffold.ts` (+8 / -116 net)
- `packages/codev/src/commands/init.ts` (+1 / -1)
- `packages/codev/src/commands/adopt.ts` (+1 / -1)
- `packages/codev/src/commands/update.ts` (+3 / -1)
- `packages/codev/src/__tests__/gitignore.test.ts` (+233 / -0, new)
- `packages/codev/src/__tests__/scaffold.test.ts` (+0 / -209)
- `codev/plans/882-refactor-extract-gitignore-hel.md` (+82 / -0, plan artifact)
- `codev/state/pir-882_thread.md` (+24 / -0, thread artifact)

## Commits

- `27f055d8` [PIR #882] Plan draft
- `63689fac` [PIR #882] Extract gitignore helpers out of scaffold.ts
- `15418d71` [PIR #882] Thread: implementation complete

(Plus six `chore(porch)` commits porch wrote at phase / gate transitions — these document the protocol's state-machine moves but contain no code changes.)

## Test Results

- `pnpm build` (root, builds `@cluesmith/codev-core` then `@cluesmith/codev`): ✓ pass
- `pnpm test -- run` (full vitest suite): ✓ pass — **151 files, 3187 tests, 0 failed, 13 pre-existing skips**
- Porch's `dev-approval` `checks` block re-ran `build` (5.5s) and `tests` (20.2s) at `porch done`: both green
- Grep audit (`createGitignore | updateGitignore | backfillGitignore | CODEV_GITIGNORE_ENTRIES | FULL_GITIGNORE_CONTENT | parseEntryLines | BackfillGitignoreResult | BackfillGitignoreOptions | UpdateGitignoreResult`): zero hits in `scaffold.ts` / `scaffold.test.ts`; all consumers reach the new module via `../lib/gitignore.js`
- Manual verification: the human approved the running worktree at the `dev-approval` gate

## Architecture Updates

No arch.md changes needed — this PR reorganizes file boundaries within `packages/codev/src/lib/` without changing module responsibilities, public CLI surface, or any cross-package contract. The new `gitignore.ts` is a peer of `scaffold.ts` in the same `lib/` folder, consumed by the same three commands. No new layer, no new dependency direction, no new pattern. The arch doc's existing description of init / adopt / update remains accurate.

## Lessons Learned Updates

No lessons-learned.md changes needed — this was a mechanical extraction predicated on a clear smell (filename no longer matches contents) that the issue itself articulated. The decision to split was already made and validated by the post-merge discussion on PR #881; the execution carried no surprises worth capturing as durable wisdom. The general pattern ("watch for header drift; rename or split when a file's name stops matching its contents") is well-known and not specific to this codebase.

## Things to Look At During PR Review

- **Test split fidelity** (`gitignore.test.ts` vs the deleted blocks from `scaffold.test.ts`): the four `describe` blocks (`createGitignore`, `updateGitignore`, `CODEV_GITIGNORE_ENTRIES`, `backfillGitignore (issue #880)`) moved verbatim — same `it()` bodies, same assertions. The new file ships its own minimal `beforeEach` (just `tempDir` — the scaffold-only skeleton fixtures aren't needed here) instead of inheriting `scaffold.test.ts`'s heavier setup. Worth diffing the moved blocks line-by-line if you want to confirm zero behavioral drift in the tests themselves.
- **Spec 0126 regression block stayed in `scaffold.test.ts`** (lines 274–) — it reads `scaffold.ts` source directly, so it correctly belongs with the scaffold tests. I considered adding a parallel regression that asserts `scaffold.ts` source no longer contains `gitignore` / `CODEV_GITIGNORE_ENTRIES`-style strings (mirroring the projectlist pattern), and flagged the option in the plan, but deferred the inclusion decision to dev-approval review and ultimately did not add it. Cheap to drop in if you want it.
- **Import shape in the three command files**: each command was changed from a single `from '../lib/scaffold.js'` block to two blocks — scaffold helpers from `scaffold.js`, gitignore helpers from `gitignore.js`. Symmetric with the existing pattern elsewhere in the codebase.
- **`parseEntryLines` stayed module-private**, same as before. No tests reach it directly — only via `backfillGitignore`.

## How to Test Locally

For reviewers pulling the branch:

- **View diff**: VSCode sidebar → right-click builder `pir-882` → **View Diff** (auto-detects the repo's default branch)
- **Run dev server**: this PR has no UI surface, so the dev server doesn't add information — but `afx dev pir-882` works if you want to smoke-test the CLI from inside the worktree
- **What to verify**:
- `pnpm build` clean (catches any missed import)
- `pnpm test -- run gitignore scaffold init adopt update` green — the rewired commands exercise the moved imports end-to-end (init writes a fresh `.gitignore`, adopt merges into existing, update backfills missing entries)
- From the built CLI: `node packages/codev/dist/cli.js init /tmp/codev-smoke --yes && cat /tmp/codev-smoke/.gitignore` — expect the Codev block including `.architect-role.md` and the standard `node_modules/` / `dist/` / OS-file entries
24 changes: 24 additions & 0 deletions codev/state/pir-882_thread.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# pir-882 thread

## 2026-05-27 — Plan drafted

Refactor: extract gitignore helpers (`CODEV_GITIGNORE_ENTRIES`, `FULL_GITIGNORE_CONTENT`, `createGitignore`, `updateGitignore`, `backfillGitignore`, `parseEntryLines`, related types) out of `scaffold.ts` into a new `packages/codev/src/lib/gitignore.ts`. Pure file move + import rewire across `init.ts`, `adopt.ts`, `update.ts`, and a test split (new `__tests__/gitignore.test.ts` carrying the moved `describe` blocks verbatim).

Grep audit confirmed the consumer list is exhaustive — three command files plus `__tests__/scaffold.test.ts`. No external consumers, so no re-exports needed from `scaffold.ts`.

Plan at `codev/plans/882-refactor-extract-gitignore-hel.md`. Awaiting plan-approval gate.

## 2026-05-27 — Implementation complete

Plan-approval approved. Implemented the refactor as planned (commit `63689fac`):

- Created `packages/codev/src/lib/gitignore.ts` with all gitignore exports + the internal `parseEntryLines` helper.
- Trimmed `packages/codev/src/lib/scaffold.ts` to genuine scaffolding helpers; updated its module header.
- Split imports in `init.ts`, `adopt.ts`, `update.ts` — gitignore symbols now come from `../lib/gitignore.js`.
- Moved the four gitignore-related `describe` blocks from `scaffold.test.ts` to a new `__tests__/gitignore.test.ts` with its own minimal fixture setup. Kept the Spec 0126 `projectlist removal` regression block in `scaffold.test.ts`.

**Verification**: `pnpm build` clean (full monorepo). Full vitest suite green — 151 files, 3187 passed, 13 skipped, 0 failed. Grep audit confirms zero gitignore references remain in `scaffold.ts` / `scaffold.test.ts`.

Did not include the optional regression test (asserting `scaffold.ts` no longer mentions gitignore) — deferring per the plan's "decision deferred to dev-approval review."

Awaiting dev-approval gate.
Loading
Loading