Skip to content

Extract shared engine/pai-paths.ts: deduplicate getPaiHomeDir + cpFilter across three migrators (#160 follow-up) #162

@virtualian

Description

@virtualian

Background

While reviewing #160 (SecurityValidator runtime materialisation), `/simplify` flagged HIGH-severity reuse findings: three migration modules each inline the same `PAI_DIR` resolver and the same junk-file ignore filter.

Duplication

`getPaiHomeDir` resolver — 7-line env-var massaging duplicated 3×

File Lines Returns
`Releases/v4.0.3+/.claude/PAI-Install/engine/skill-migration.ts` 87-96 `join(paiHome, "skills")` (named `getPaiSkillsDir`)
`Releases/v4.0.3+/.claude/PAI-Install/engine/command-migration.ts` 76-85 `join(paiHome, "commands")` (named `getPaiCommandsDir`)
`Releases/v4.0.3+/.claude/PAI-Install/engine/pai-runtime-migration.ts` 69-77 `paiHome` directly (named `getPaiHomeDir`)

The env-resolution preamble (handling ``, `/`, `$HOME` prefixes; falling back to `join(homedir(), ".pai")`) is byte-identical across all three. Only the trailing `join` differs.

Severity: HIGH. Any bug fix to PAI_DIR parsing (e.g. handling `${HOME}` literal syntax, trailing slashes, `~user` patterns) must land in three places.

Junk-file filter — duplicated 3×

File Lines
`skill-migration.ts` 64-76, 119-122 (`IGNORED_ENTRIES`, `isIgnored`, `cpFilter`, `BACKUP_SUFFIX_RE`)
`command-migration.ts` 61-69 (same plus `.gitkeep`; `BACKUP_SUFFIX_RE` same)
`pai-runtime-migration.ts` 42-56 (`IGNORED_ENTRIES`, `isIgnored`, `cpFilter`; no `BACKUP_SUFFIX_RE` — its scope creates no backups)

Drift already exists: `command-migration.ts` adds `.gitkeep`; `pai-runtime-migration.ts` legitimately omits backup-suffix matching. The divergence is silent — a maintainer adding a new junk pattern (e.g. `.idea/`) would have to know to update three files.

Severity: HIGH (drift potential).

Proposed refactor

New module `Releases/v4.0.3+/.claude/PAI-Install/engine/pai-paths.ts`:

```ts
export function getPaiHome(): string { /* the shared 7-line env resolver */ }

export const getPaiSkillsDir = () => join(getPaiHome(), "skills");
export const getPaiCommandsDir = () => join(getPaiHome(), "commands");

export function isJunkEntry(name: string, opts?: { allowBackups?: boolean }): boolean {
// .DS_Store + ._* + (optionally) backup-timestamp suffix
}

export const cpFilterDefault = (src: string) => !isJunkEntry(basename(src));
```

Each migration file deletes its local copies and imports from `pai-paths.ts`. `__testInternals` exports stay backward-compatible (re-export imported symbols).

Acceptance criteria

  • New `engine/pai-paths.ts` module with shared resolver + filter.
  • All three migrators (`skill-migration.ts`, `command-migration.ts`, `pai-runtime-migration.ts`) consume the shared module instead of inlining.
  • Existing tests pass (`skill-migration.test.ts`, `memory-migration.test.ts`).
  • `Tools/verify-security-validator.sh` still PASS=8 FAIL=0 after refactor.
  • No behaviour change — pure deduplication.

Refs

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions