diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index 55d27956e..ef277c125 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -153,7 +153,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/commands/book-mirror.ts` — `gbrain book-mirror --chapters-dir --slug [flags]`. Submits N read-only subagent jobs (one per chapter; `allowed_tools: ['get_page', 'search']`), waits for all via `waitForCompletion`, reads each child's `job.result`, assembles two-column markdown CLI-side, writes a single operator-trust `put_page` to `media/books/-personalized.md`. Trust narrowing happens at the tool-allowlist layer (subagents can't call put_page) so untrusted EPUB content can't prompt-inject any people page. Cost-estimate prompt before launching; refuses to spend in non-TTY without `--yes`. Per-chapter idempotency keys (`book-mirror::ch-`) for retry-friendly re-runs. Partial-failure: assembles completed chapters + a `## Failed chapters` section. Pinned by `test/book-mirror.test.ts` (9 cases). - `src/commands/skillpack.ts` + `src/core/skillpack/{bundle,scaffold,reference,migrate-fence,scrub-legacy,harvest,harvest-lint,copy,apply-hunks,diff-text,installer}.ts` — managed-block install model retired; `install`/`uninstall` removed (exit non-zero with a hint to the replacement). Surface: `scaffold` (one-time additive copy via `copyArtifacts` in `copy.ts`; refuses to overwrite; partial-state fills missing paired sources declared in SKILL.md frontmatter `sources:`), `reference` (read-only diff lens + `--apply-clean-hunks` two-way auto-apply via pure-JS unified-diff parser/applier in `apply-hunks.ts` + `diff-text.ts`), `migrate-fence` (one-shot strip of legacy fence; cumulative-slugs receipt → row-parsing fallback; preserves rows verbatim as user-owned routing), `scrub-legacy-fence-rows` (opt-in row cleanup with skill-present + non-empty-triggers gate), `harvest` (host→gbrain inverse with symlink-reject + canonical-path containment via a `validateUploadPath`-style gate + default-on privacy linter in `harvest-lint.ts` against `~/.gbrain/harvest-private-patterns.txt` plus built-in a built-in fork-name pattern + email + Slack-channel patterns; rollback on match). Paired-source declarations live in each SKILL.md's frontmatter `sources:` array (validated by `loadSkillSources` in `bundle.ts`). `autoDetectSkillsDir` (in `src/core/repo-root.ts`) has a `cwd_walk_up` tier ahead of `~/.openclaw/workspace` (`$OPENCLAW_WORKSPACE` precedence preserved). `gbrain skillpack check --strict` exits non-zero on drift (CI gate); top-level `gbrain skillpack-check` keeps exit-1-on-issues for cron. Companion editorial skill `skills/skillpack-harvest/SKILL.md` drives the genericization checklist. Doc: `docs/guides/skillpacks-as-scaffolding.md`. Test coverage across `test/skillpack-{copy,scaffold,reference,reference-apply,apply-hunks,migrate-fence,scrub-legacy,harvest,harvest-lint,frontmatter-sources}.test.ts` + 9-case E2E in `test/e2e/skillpack-flow.test.ts`. `installer.ts` + `test/skillpack-install.test.ts` survive — `gbrain skillpack diff` still uses `diffSkill` from there. - `src/core/skillpack/{manifest-v1,tarball,state,remote-source,trust-prompt,bootstrap-display,scaffold-third-party,registry-schema,registry-client,rubric,doctor,init-scaffold,pack-publish,endorse,audit}.ts` + `examples/skillpack-reference/` + `docs/skillpack-anatomy.md` + `scripts/build-skillpack-anatomy.ts` — third-party skillpack ecosystem. `gbrain skillpack scaffold ` resolves the spec via `classifySpec`, fetches through SSRF-hardened `git-remote.ts` (git) or extracts the tarball into `~/.gbrain/skillpack-cache/////`, validates `skillpack.json` (api_version `gbrain-skillpack-v1`), checks `gbrain_min_version`, surfaces a TOFU first-install identity-confirm prompt (author + source + pinned commit + tarball SHA + tier; non-TTY requires `--trust`), records the pin in machine-owned `~/.gbrain/skillpack-state.json` (schema `gbrain-skillpack-state-v1`, atomic `.tmp + rename`, `isAlreadyTrusted` skips re-prompt on author+pin match), runs through `enumerateScaffoldEntries` → `copyArtifacts` (one-time additive, refuses to overwrite), then DISPLAYS `runbooks/bootstrap.md` WITHOUT executing (deliberately does not auto-execute). Registry catalog at `garrytan/gbrain-skillpack-registry` split into `registry.json` (PR-able, `gbrain-registry-v1`) + `endorsements.json` (Garry-only overlay, `gbrain-endorsements-v1`); `effectiveTier` merges. `registry-client.ts` fetches both via `If-None-Match` etag with 1h soft-TTL + stale-fallback (origins `fresh_fetch | cache_warm | cache_soft_stale | cache_hard_stale`); hard-fail only on no-cache + no-network. CLI: `gbrain skillpack {search,info,registry,doctor,init,pack,endorse}`. Doctor walks `SKILLPACK_RUBRIC_V1` (10 binary dimensions: 5 required CORE — manifest_valid, skills_have_skill_md, routing_evals_present ≥5 intents, skills_have_unique_triggers MECE, changelog_present_and_current — and 5 quality BADGES — unit_tests_present, e2e_tests_present, llm_eval_present ≥3 cases, bootstrap_runbook_present, license_present); tier eligibility: `endorsed` needs all 10, `community` needs core + ≥3 badges, `experimental` needs core only, `blocked` when any core fails. `--quick` ~5s structural sweep; `--fix --yes` auto-scaffolds `auto_fixable: true` dimensions and refuses to overwrite files whose mtime is newer than `skillpack.json`. `gbrain skillpack init ` lands 11 files (skillpack.json, SKILL.md, routing-eval.jsonl, test/example.test.ts, e2e/example.e2e.test.ts, evals/example.judge.json, runbooks/{bootstrap,uninstall,upgrade-template}.md, CHANGELOG, README, LICENSE); freshly-init'd scores 10/10; `--minimal` skips test/e2e/evals. `gbrain skillpack pack` packs a deterministic tarball via GNU tar (`--sort=name --mtime=@0 --owner=0 --group=0 --numeric-owner` + `GZIP=-n` + `TZ=UTC`); refuses on `tier_eligibility === 'blocked'`. Extract caps (5000 files / 100MB total / 1MB per file / 255-char paths / 100:1 compression ratio); rejects symlinks/hardlinks/devices/FIFOs. `gbrain skillpack endorse [--tier ...] [--push] [--dry-run]` runs in a clone of the registry repo: validates the pack in `registry.json`, mutates `endorsements.json` via pure `applyEndorsement`, stable-key-orders the write, commits `endorse: -> `, optionally pushes. JSONL audit at `~/.gbrain/audit/skillpack-YYYY-Www.jsonl` (ISO-week rotated, honors `GBRAIN_AUDIT_DIR`). `examples/skillpack-reference/` is a 10/10 reference pack pinned by `test/e2e/skillpack-third-party.test.ts`. `docs/skillpack-anatomy.md` auto-generated via `scripts/build-skillpack-anatomy.ts` (`--check` for CI drift). CLI dispatch in `src/commands/skillpack.ts` disambiguates third-party (contains `/`, `://`, `.tgz`) from bundled-skill kebab; kebab routes bundled-first, registry-fallback. Tests: `test/skillpack-{manifest-v1,tarball,state,remote-source,trust-prompt,registry-schema,registry-client,rubric,doctor,init-scaffold,pack-publish,endorse,audit,scaffold-third-party}.test.ts` + `test/e2e/skillpack-third-party.test.ts`. Spec at `docs/designs/SKILLPACK_REGISTRY_V1_SPEC.md`. -- `src/core/archive-crawler-config.ts` — safety gate for the `archive-crawler` skill. Refuses to run unless `archive-crawler.scan_paths:` is explicitly set in the brain repo's `gbrain.yml`. Mirrors the storage-config.ts parsing pattern (sibling file, separate concern from storage tiering). `loadArchiveCrawlerConfig(repoPath)` throws `ArchiveCrawlerConfigError(missing_section | empty_scan_paths | invalid_path | parse_error)`. `normalizeAndValidateArchiveCrawlerConfig` rejects relative paths and `..` traversal; `~` is expanded; trailing-slash normalized for unambiguous prefix matching. `isPathAllowed(candidate, config)` is the runtime per-file gate (scan_paths prefix-match with directory-boundary correctness; deny_paths overrides). Pinned by `test/archive-crawler-config.test.ts` (19 cases). +- `src/core/archive-crawler-config.ts` — safety gate for the `archive-crawler` skill. Refuses to run unless `archive-crawler.scan_paths:` is explicitly set in the brain repo's `gbrain.yml`. Mirrors the storage-config.ts parsing pattern (sibling file, separate concern from storage tiering). `loadArchiveCrawlerConfig(repoPath)` throws `ArchiveCrawlerConfigError(missing_section | empty_scan_paths | invalid_path | parse_error)`. `normalizeAndValidateArchiveCrawlerConfig` rejects relative paths and `..` traversal; `~` is expanded; paths are stored resolved and terminated with the PLATFORM separator (`path.sep`) so error output reads natively on each OS. `isPathAllowed(candidate, config)` is the runtime per-file gate (scan_paths prefix-match with directory-boundary correctness; deny_paths overrides). Candidate, scan_paths and deny_paths all funnel through the private `toComparablePrefix()` before the prefix test — on Windows it folds `\`→`/` and lowercases (NTFS is case-insensitive, so a deny_path spelled `Private` must still match `private`, else the gate fails OPEN); on POSIX it is identity apart from the trailing separator, deliberately NOT folding, since `\` is a legal filename character and paths are case-sensitive. Storing a native separator while appending a hardcoded `/` is the mixed-separator bug that made `isPathAllowed` deny every real path on Windows; the two functions must stay symmetric or the prefix test is meaningless. Pinned by `test/archive-crawler-config.test.ts` (26 cases, platform-selected fixtures + `it.if`-gated win32/POSIX comparison semantics). - `test/helpers/cli-pty-runner.ts` — generic real-PTY harness (~470 lines) using pure `Bun.spawn({terminal:})` (Bun 1.3.10+; engines.bun pin in package.json). Generic primitives only, no plan-mode orchestrators. Exports `launchPty`, `resolveBinary`, `stripAnsi`, `parseNumberedOptions`, `optionsSignature`, `isNumberedOptionListVisible`, `isTrustDialogVisible`. Self-tests in `test/cli-pty-runner.test.ts` (24 cases). - `src/core/skillpack/{init-brain-pack,brain-pack-advisory,brain-pack-lint,brain-resident-locate,nag-state}.ts` (#2180) — brain-resident skillpacks. `manifest-v1.ts` gains optional `brain_resident` + `schema_pack` (additive). `runInitBrainPack` scaffolds a pack beside brain content (`brain_resident:true`, exact `gbrain_min_version`, 5-section machine-parseable README); `applyWritePlan` is factored out of `init-scaffold.ts` for the shared refuse-overwrite loop. `brain-pack-lint.lintBrainPackTools` validates each skill's `tools:` against the serving op set (E6 version-skew). Topology A: `src/commands/sources.ts` `runAdd` prints `brain-pack-advisory` to stderr after `opsAddSource`, fail-open; `nag-state.ts` (`~/.gbrain/skillpack-nag-state.json`) keys declines by `(source-repo brain_id, source_id, pack_name)` with pure `decideNagAction` (first/reminder/version-bump/ceiling) — declines count ONLY on CLI-interactive displays. Topology B: `brain-resident-locate.loadResidentPacksForServer` (source-scoped via `sourceScopeOpts`) backs the `list_brain_skillpack` op; `getResidentSkillDetail` backs `get_skill` `source_id`; `scaffold_spec` is the git source, never a server FS path. Tests: `test/skillpack-{init-brain-pack,nag-state,brain-resident-locate}.test.ts` + the brain-resident cases in `test/skillpack-manifest-v1.test.ts`. - `src/core/advisor/{types,run,render,recommended-set,history,apply,collect-*}.ts` + `src/commands/advisor.ts` (#2180) — `gbrain advisor`: read-only ranked actions from brain state. `run.runAdvisor` executes 8 hardcoded collectors (version [cache-only], migration, schema-pack, stalled-jobs [absent-table tolerant], usage-shape, setup-smells, uninstalled-brain-pack, uninstalled-bundled), each in its own try/catch; `rankFindings` orders critical>warn>info then collector order, caps the info tail, and drops `workspace_dependent` findings when `remote` (A1). `render.ts` is the shared `=`-bar renderer used by the advisor AND `post-install-advisory.ts` (generalized to a single current-state `recommended-set.RECOMMENDED`, `install`→`scaffold`). `history.ts` appends bounded `~/.gbrain/advisor-history.jsonl` (no DB migration) for since-last-run deltas; local-only. `apply.resolveApplyTarget` is the allowlist+injection guard for `commands/advisor.ts --apply ` (structured argv, never a shell; local-only). The `advisor` op (`operations.ts`) is read-scoped, NOT localOnly, gated by `mcp.publish_advisor` (config.ts; default off) and strictly read-only on remote. CLI wired in `cli.ts` (`CLI_ONLY` + dispatch). Bundled skill `skills/gbrain-advisor/` + weekly cron recipe. Tests: `test/advisor-{core,apply,op-gate,ranking-eval}.test.ts`. diff --git a/src/core/archive-crawler-config.ts b/src/core/archive-crawler-config.ts index e4235cfbe..8cceaf513 100644 --- a/src/core/archive-crawler-config.ts +++ b/src/core/archive-crawler-config.ts @@ -29,11 +29,12 @@ import { existsSync, readFileSync } from 'fs'; import { homedir } from 'os'; -import { isAbsolute, join, resolve as resolvePath } from 'path'; +import { isAbsolute, join, sep, resolve as resolvePath } from 'path'; export interface ArchiveCrawlerConfig { /** Absolute paths the agent is permitted to scan. ~ expanded; paths - * normalized to absolute form; trailing-slash normalized. + * normalized to absolute form; terminated with the PLATFORM separator + * (`\` on Windows, `/` on POSIX). * Required to be non-empty when the section exists. */ scan_paths: string[]; /** Absolute paths within scan_paths to explicitly deny. Optional; @@ -134,6 +135,48 @@ function expandHome(p: string): string { return p; } +const IS_WINDOWS = process.platform === 'win32'; + +/** + * toComparablePrefix — the ONE canonical form used for every prefix + * comparison in this module. Both the stored allow/deny paths and the + * candidate path must go through this, or the prefix test is + * meaningless. + * + * Stored paths keep native separators (see normalizeOnePath) so error + * messages and CLI output read naturally on each platform; this + * function exists so the *comparison* is separator- and case-agnostic + * without the stored form having to be. + * + * On Windows: + * - `\` is folded to `/`. `resolve()` emits `\`, but a user may + * legitimately write `C:/Users/...` in gbrain.yml (Win32 accepts + * both), and a caller may hand-build a config with either. Mixing + * the two at the boundary is precisely the bug this replaces: a + * `\`-joined candidate never matched a `/`-terminated prefix, so + * isPathAllowed denied every real path. + * - The result is lowercased. NTFS is case-insensitive, so + * `...\writing\Private\` and `...\writing\private\` are the SAME + * directory. A case-sensitive compare would let + * `...\writing\private\tax.md` slip past a deny_path spelled + * `Private` — a fail-OPEN against exactly the sensitive content + * D12 exists to fence off. + * + * On POSIX: identity apart from the trailing separator. Deliberately + * NOT folded — `\` is a legal filename character and paths are + * case-sensitive, so folding either would collide two genuinely + * different paths into one comparable and fail open. + * + * The trailing `/` is appended AFTER folding so a single check covers + * both a native-separator tail (`C:\a\b\`) and an already-forward + * -slashed one, keeping directory-boundary matching intact + * (`/a/b/` must not match `/a/bc/`). + */ +function toComparablePrefix(p: string): string { + const folded = IS_WINDOWS ? p.replace(/\\/g, '/').toLowerCase() : p; + return folded.endsWith('/') ? folded : folded + '/'; +} + /** * Normalize and validate a parsed RawArchiveCrawler into the public * ArchiveCrawlerConfig shape. @@ -147,9 +190,11 @@ function expandHome(p: string): string { * - Path-traversal rejection: a path containing `..` after * normalization is rejected to prevent allow-list escape via * `~/Documents/../../../etc/passwd`. Throws invalid_path. - * - Trailing slash normalization: paths without trailing slash get - * one appended (so prefix matching is unambiguous: `/a/b/` - * does NOT match `/a/bc/`). + * - Trailing separator normalization: paths without a trailing + * separator get the PLATFORM separator appended (so prefix matching + * is unambiguous: `/a/b/` does NOT match `/a/bc/`). Separator- and + * case-folding for the comparison itself lives in + * toComparablePrefix(), which isPathAllowed() applies to both sides. */ export function normalizeAndValidateArchiveCrawlerConfig( raw: RawArchiveCrawler, @@ -194,10 +239,16 @@ function normalizeOnePath(raw: string, field: 'scan_paths' | 'deny_paths'): stri ); } - // Normalize: resolve any tail and ensure trailing slash for unambiguous - // prefix-matching. resolve() strips trailing slash; we re-add it. + // Normalize: resolve any tail and ensure a trailing separator for + // unambiguous prefix-matching. resolve() strips the trailing separator; + // we re-add it using the PLATFORM separator, not a hardcoded '/' — + // resolve() emits '\' on Windows, so appending '/' produced a + // mixed-separator path ('C:\Users\...\writing/') that no candidate + // could ever prefix-match. Comparison is done on the folded form from + // toComparablePrefix(), so storing native separators here is safe and + // keeps error messages readable on each platform. const resolved = resolvePath(expanded); - return resolved.endsWith('/') ? resolved : resolved + '/'; + return resolved.endsWith(sep) ? resolved : resolved + sep; } /** @@ -267,8 +318,15 @@ export function loadArchiveCrawlerConfig( * (when it grows a runtime check) to gate per-file decisions. * * Both inputs are normalized via `resolvePath` and compared as absolute - * directory prefixes (with trailing slash) so `media/x/` does not match - * `media/xerox/foo`. + * directory prefixes (with trailing separator) so `media/x/` does not + * match `media/xerox/foo`. + * + * Every side of the comparison — candidate, scan_paths and deny_paths — + * is funnelled through toComparablePrefix() so the three agree on + * separator and case. The config entries are folded here rather than + * trusted as-is because isPathAllowed is part of the public surface and + * a caller may hand-build an ArchiveCrawlerConfig without going through + * normalizeAndValidateArchiveCrawlerConfig(). */ export function isPathAllowed( candidate: string, @@ -276,14 +334,17 @@ export function isPathAllowed( ): boolean { const expanded = expandHome(candidate); if (!isAbsolute(expanded)) return false; - const resolved = resolvePath(expanded); - const prefix = resolved.endsWith('/') ? resolved : resolved + '/'; + const prefix = toComparablePrefix(resolvePath(expanded)); // Must be inside at least one scan_path. - const allowed = config.scan_paths.some((sp) => prefix.startsWith(sp)); + const allowed = config.scan_paths.some((sp) => + prefix.startsWith(toComparablePrefix(sp)), + ); if (!allowed) return false; // Must NOT be inside any deny_path. - const denied = config.deny_paths.some((dp) => prefix.startsWith(dp)); + const denied = config.deny_paths.some((dp) => + prefix.startsWith(toComparablePrefix(dp)), + ); return !denied; } diff --git a/src/core/skillpack/copy.ts b/src/core/skillpack/copy.ts index 100342ad3..fcbcb3768 100644 --- a/src/core/skillpack/copy.ts +++ b/src/core/skillpack/copy.ts @@ -15,7 +15,7 @@ * gets a chance to copy, or nothing does. */ import { existsSync, lstatSync, mkdirSync, readdirSync, readFileSync, realpathSync, writeFileSync } from 'fs'; -import { dirname, join, relative } from 'path'; +import { dirname, join, relative, sep } from 'path'; export interface CopyItem { /** Absolute source path. */ @@ -153,7 +153,10 @@ export function copyArtifacts(items: CopyItem[], opts: CopyArtifactsOpts = {}): const real = realpathSync(item.source); // realpathSync returns paths without trailing slash; add path // separator to the prefix check so /a/b doesn't match /a/bb. - const prefix = confineRoot.endsWith('/') ? confineRoot : confineRoot + '/'; + // Both sides are realpathSync() output, so the separator must be the + // NATIVE one — a hardcoded '/' never matches a win32 path and rejected + // every source as path_traversal. + const prefix = confineRoot.endsWith(sep) ? confineRoot : confineRoot + sep; if (real !== confineRoot && !real.startsWith(prefix)) { throw new CopyError( `${item.source}: path traversal rejected. Source canonicalizes outside the confinement root (${confineRoot}).`, diff --git a/test/archive-crawler-config.test.ts b/test/archive-crawler-config.test.ts index 5d64a82a2..404dc3f4f 100644 --- a/test/archive-crawler-config.test.ts +++ b/test/archive-crawler-config.test.ts @@ -9,15 +9,27 @@ * - empty scan_paths -> empty_scan_paths * - relative path -> invalid_path * - path traversal (..) -> invalid_path - * - valid config -> normalized absolute trailing-slashed paths + * - valid config -> normalized absolute trailing-separator paths * - ~ expansion * - deny_paths optional * - isPathAllowed: prefix match + deny override + prefix boundary + * + * PLATFORM NOTE: these tests must run on both POSIX (gbrain CI is + * 100% ubuntu-latest) and Windows. POSIX path literals cannot be + * shared across both, because `/home/user` is NOT a drive-qualified + * absolute path on Windows — `path.resolve('/home/user')` returns + * `C:\home\user` (the cwd's drive). That is correct Win32 semantics, + * not a product bug, so the FIXTURES are platform-selected via ROOT + * rather than the product being forced to emit POSIX paths. + * + * Only the separator character is taken from `path`; the directory + * structure in every expectation is still hand-written, so these stay + * real assertions rather than a tautological re-run of the impl. */ import { describe, expect, it, beforeEach, afterEach } from 'bun:test'; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs'; -import { join } from 'path'; +import { mkdtempSync, rmSync, writeFileSync } from 'fs'; +import { join, sep } from 'path'; import { homedir, tmpdir } from 'os'; import { loadArchiveCrawlerConfig, @@ -26,6 +38,14 @@ import { ArchiveCrawlerConfigError, } from '../src/core/archive-crawler-config.ts'; +const WIN = process.platform === 'win32'; + +/** Absolute-path fixture root for the current platform. */ +const ROOT = WIN ? 'C:\\gbtest' : '/home/user'; + +/** Shorthand for the platform separator used in expectations. */ +const S = sep; + let workdir: string; beforeEach(() => { @@ -79,7 +99,7 @@ describe('loadArchiveCrawlerConfig — D12 missing_section', () => { describe('loadArchiveCrawlerConfig — D12 empty_scan_paths', () => { it('throws empty_scan_paths when scan_paths is omitted', () => { - writeYaml('archive-crawler:\n deny_paths:\n - /tmp/forbidden/\n'); + writeYaml(`archive-crawler:\n deny_paths:\n - ${join(ROOT, 'forbidden')}\n`); expect(() => loadArchiveCrawlerConfig(workdir)).toThrow(ArchiveCrawlerConfigError); try { loadArchiveCrawlerConfig(workdir); @@ -106,7 +126,9 @@ describe('loadArchiveCrawlerConfig — D12 invalid_path', () => { }); it('throws invalid_path on path traversal (..)', () => { - writeYaml('archive-crawler:\n scan_paths:\n - /home/user/Documents/../../etc/passwd\n'); + writeYaml( + `archive-crawler:\n scan_paths:\n - ${join(ROOT, 'Documents')}${S}..${S}..${S}etc${S}passwd\n`, + ); try { loadArchiveCrawlerConfig(workdir); throw new Error('expected throw'); @@ -118,9 +140,9 @@ describe('loadArchiveCrawlerConfig — D12 invalid_path', () => { it('rejects ".." in deny_paths too', () => { writeYaml(`archive-crawler: scan_paths: - - /home/user/Documents/ + - ${join(ROOT, 'Documents')} deny_paths: - - /home/user/Documents/../etc + - ${join(ROOT, 'Documents')}${S}..${S}etc `); try { loadArchiveCrawlerConfig(workdir); @@ -132,16 +154,16 @@ describe('loadArchiveCrawlerConfig — D12 invalid_path', () => { }); describe('loadArchiveCrawlerConfig — happy path', () => { - it('returns normalized absolute paths with trailing slash', () => { + it('returns normalized absolute paths with trailing separator', () => { writeYaml(`archive-crawler: scan_paths: - - /home/user/writing - - /mnt/backup/old-letters/ + - ${join(ROOT, 'writing')} + - ${join(ROOT, 'backup', 'old-letters')}${S} `); const config = loadArchiveCrawlerConfig(workdir); expect(config.scan_paths).toEqual([ - '/home/user/writing/', - '/mnt/backup/old-letters/', + `${ROOT}${S}writing${S}`, + `${ROOT}${S}backup${S}old-letters${S}`, ]); expect(config.deny_paths).toEqual([]); }); @@ -150,28 +172,28 @@ describe('loadArchiveCrawlerConfig — happy path', () => { const home = homedir(); writeYaml('archive-crawler:\n scan_paths:\n - ~/Documents/writing\n'); const config = loadArchiveCrawlerConfig(workdir); - expect(config.scan_paths[0]).toBe(`${home}/Documents/writing/`); + expect(config.scan_paths[0]).toBe(`${home}${S}Documents${S}writing${S}`); }); it('accepts deny_paths alongside scan_paths', () => { writeYaml(`archive-crawler: scan_paths: - - /home/user/Documents/ + - ${join(ROOT, 'Documents')}${S} deny_paths: - - /home/user/Documents/finances/ - - /home/user/Documents/medical/ + - ${join(ROOT, 'Documents', 'finances')}${S} + - ${join(ROOT, 'Documents', 'medical')}${S} `); const config = loadArchiveCrawlerConfig(workdir); expect(config.deny_paths).toEqual([ - '/home/user/Documents/finances/', - '/home/user/Documents/medical/', + `${ROOT}${S}Documents${S}finances${S}`, + `${ROOT}${S}Documents${S}medical${S}`, ]); }); it('accepts both archive-crawler and archive_crawler key spellings', () => { - writeYaml('archive_crawler:\n scan_paths:\n - /home/user/notes\n'); + writeYaml(`archive_crawler:\n scan_paths:\n - ${join(ROOT, 'notes')}\n`); const config = loadArchiveCrawlerConfig(workdir); - expect(config.scan_paths[0]).toBe('/home/user/notes/'); + expect(config.scan_paths[0]).toBe(`${ROOT}${S}notes${S}`); }); }); @@ -182,43 +204,125 @@ describe('normalizeAndValidateArchiveCrawlerConfig — direct API', () => { ); }); - it('returns trailing-slashed normalized paths', () => { + it('returns trailing-separator normalized paths', () => { const out = normalizeAndValidateArchiveCrawlerConfig({ - scan_paths: ['/a/b', '/c/d/'], + scan_paths: [join(ROOT, 'a', 'b'), `${join(ROOT, 'c', 'd')}${S}`], }); - expect(out.scan_paths).toEqual(['/a/b/', '/c/d/']); + expect(out.scan_paths).toEqual([ + `${ROOT}${S}a${S}b${S}`, + `${ROOT}${S}c${S}d${S}`, + ]); + }); + + it('terminates every normalized path with the PLATFORM separator, not a foreign one', () => { + // Regression pin for the mixed-separator bug: resolve() emits '\' on + // Windows but the old code appended a hardcoded '/', producing + // 'C:\Users\...\writing/'. isPathAllowed then compared a '\'-joined + // candidate against that '/'-terminated prefix and never matched. + const out = normalizeAndValidateArchiveCrawlerConfig({ + scan_paths: [join(ROOT, 'writing')], + deny_paths: [join(ROOT, 'writing', 'private')], + }); + for (const p of [...out.scan_paths, ...out.deny_paths]) { + expect(p.endsWith(S)).toBe(true); + if (WIN) { + // No stray forward slash anywhere: the whole path is native-separator. + expect(p.includes('/')).toBe(false); + } + } }); }); -describe('isPathAllowed', () => { - const config = { - scan_paths: ['/home/user/writing/', '/home/user/Dropbox/'], - deny_paths: ['/home/user/Dropbox/finances/'], - }; - - it('returns true for a path inside a scan_path', () => { - expect(isPathAllowed('/home/user/writing/essay.md', config)).toBe(true); - expect(isPathAllowed('/home/user/Dropbox/letters/a.txt', config)).toBe(true); +describe('isPathAllowed — round-trip against the real normalizer', () => { + // This is the case the original suite lacked: it hand-built the config + // from POSIX literals instead of feeding it through + // normalizeAndValidateArchiveCrawlerConfig, so the separator mismatch + // between the two functions was invisible. Four of the five old + // assertions also expected `false`, which the Windows total-deny + // satisfied by accident. + const config = normalizeAndValidateArchiveCrawlerConfig({ + scan_paths: [join(ROOT, 'writing'), join(ROOT, 'Dropbox')], + deny_paths: [join(ROOT, 'Dropbox', 'finances')], }); - it('returns false for a path outside any scan_path', () => { - expect(isPathAllowed('/etc/passwd', config)).toBe(false); - expect(isPathAllowed('/home/user/Other/thing.md', config)).toBe(false); + it('allows a file inside a scan_path', () => { + expect(isPathAllowed(join(ROOT, 'writing', 'essay.md'), config)).toBe(true); + expect(isPathAllowed(join(ROOT, 'Dropbox', 'letters', 'a.txt'), config)).toBe(true); }); - it('returns false for a path inside a deny_path even if it is also in a scan_path', () => { - expect(isPathAllowed('/home/user/Dropbox/finances/2024.pdf', config)).toBe(false); + it('allows the scan_path directory itself', () => { + expect(isPathAllowed(join(ROOT, 'writing'), config)).toBe(true); + }); + + it('denies a file outside every scan_path', () => { + expect(isPathAllowed(join(ROOT, 'Other', 'thing.md'), config)).toBe(false); + }); + + it('denies a file inside a deny_path even though it is also in a scan_path', () => { + expect(isPathAllowed(join(ROOT, 'Dropbox', 'finances', '2024.pdf'), config)).toBe(false); }); it('respects directory boundaries — /writing/ does not match /writing-stuff/', () => { - // Exact-prefix-with-trailing-slash means /home/user/writing/ does NOT - // match /home/user/writing-stuff/. This is the codex T7 / storage-config - // pattern: prefix matching at directory boundaries, not arbitrary string - // prefixes. - expect(isPathAllowed('/home/user/writing-stuff/file.md', config)).toBe(false); + // Exact-prefix-with-trailing-separator means .../writing/ does NOT + // match .../writing-stuff/. This is the codex T7 / storage-config + // pattern: prefix matching at directory boundaries, not arbitrary + // string prefixes. + expect(isPathAllowed(join(ROOT, 'writing-stuff', 'file.md'), config)).toBe(false); + }); + + it('collapses traversal in the candidate before matching', () => { + // resolve() flattens '..', so an escape attempt lands outside the + // scan_path and is denied on its resolved form. + expect(isPathAllowed(join(ROOT, 'writing', '..', 'Other', 'x.md'), config)).toBe(false); }); it('rejects relative paths', () => { expect(isPathAllowed('./relative.md', config)).toBe(false); }); }); + +describe('isPathAllowed — platform-specific comparison semantics', () => { + it.if(WIN)('win32: matches forward-slash input against native-separator config', () => { + const config = normalizeAndValidateArchiveCrawlerConfig({ + scan_paths: [join(ROOT, 'writing')], + }); + // A user may legitimately write forward slashes in gbrain.yml on + // Windows; Win32 accepts them. Comparison must not care. + expect(isPathAllowed('C:/gbtest/writing/essay.md', config)).toBe(true); + }); + + it.if(WIN)('win32: comparison is case-insensitive (NTFS semantics)', () => { + const config = normalizeAndValidateArchiveCrawlerConfig({ + scan_paths: [join(ROOT, 'writing')], + deny_paths: [join(ROOT, 'writing', 'Private')], + }); + // Drive letter case must not matter. + expect(isPathAllowed('c:\\gbtest\\writing\\essay.md', config)).toBe(true); + // And neither may the deny_path's case — this is the fail-OPEN case: + // 'Private' and 'private' are the SAME directory on NTFS, so a + // case-sensitive compare would have let the sensitive file through. + expect(isPathAllowed(join(ROOT, 'writing', 'private', 'tax.md'), config)).toBe(false); + expect(isPathAllowed(join(ROOT, 'writing', 'PRIVATE', 'tax.md'), config)).toBe(false); + }); + + it.if(!WIN)('posix: comparison stays case-sensitive', () => { + // Negative control for the win32 case-folding: POSIX filesystems are + // case-sensitive, so folding there would be a real fail-open. + const config = normalizeAndValidateArchiveCrawlerConfig({ + scan_paths: ['/home/user/Writing'], + }); + expect(isPathAllowed('/home/user/Writing/essay.md', config)).toBe(true); + expect(isPathAllowed('/home/user/writing/essay.md', config)).toBe(false); + }); + + it.if(!WIN)('posix: a literal backslash in a filename is not treated as a separator', () => { + // Backslash is a legal POSIX filename character. Folding '\' -> '/' + // on POSIX would collide two genuinely different paths, so the + // comparator must leave POSIX paths untouched. + const config = normalizeAndValidateArchiveCrawlerConfig({ + scan_paths: ['/home/user/a\\b'], + }); + expect(isPathAllowed('/home/user/a\\b/file.md', config)).toBe(true); + expect(isPathAllowed('/home/user/a/b/file.md', config)).toBe(false); + }); +}); diff --git a/test/artifact-abstraction.test.ts b/test/artifact-abstraction.test.ts index cc23d60bd..1cdad4b46 100644 --- a/test/artifact-abstraction.test.ts +++ b/test/artifact-abstraction.test.ts @@ -3,12 +3,21 @@ // so the cross-discriminator surface stays MECE. import { describe, test, expect } from 'bun:test'; +import { sep } from 'path'; import { detectArtifactKind, targetDirForKind, validateManifestByKind, } from '../src/core/artifact/index.ts'; +// targetDirForKind builds its result with path.join(), which is correct: +// it emits native separators on every platform. The fixture below is +// therefore platform-selected rather than a shared POSIX literal — on +// Windows, join('/home/u/.gbrain', 'skillpacks') is '\home\u\.gbrain\ +// skillpacks', which is right, not a bug. Only the separator comes from +// `path`; the subdirectory names being asserted are still hand-written. +const GBRAIN_HOME = process.platform === 'win32' ? 'C:\\gb' : '/home/u/.gbrain'; + describe('v0.39 T14 — artifact abstraction', () => { test('detectArtifactKind by extension', () => { expect(detectArtifactKind('/tmp/foo.gbrain-schema')).toBe('schemapack'); @@ -17,8 +26,8 @@ describe('v0.39 T14 — artifact abstraction', () => { }); test('targetDirForKind routes to distinct subdirectories', () => { - expect(targetDirForKind('schemapack', '/home/u/.gbrain')).toBe('/home/u/.gbrain/schema-packs'); - expect(targetDirForKind('skillpack', '/home/u/.gbrain')).toBe('/home/u/.gbrain/skillpacks'); + expect(targetDirForKind('schemapack', GBRAIN_HOME)).toBe(`${GBRAIN_HOME}${sep}schema-packs`); + expect(targetDirForKind('skillpack', GBRAIN_HOME)).toBe(`${GBRAIN_HOME}${sep}skillpacks`); }); test('validateManifestByKind: schemapack happy path', () => { diff --git a/test/brain-registry.serial.test.ts b/test/brain-registry.serial.test.ts index fae4e6154..e081dbae7 100644 --- a/test/brain-registry.serial.test.ts +++ b/test/brain-registry.serial.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect, afterEach } from 'bun:test'; import { mkdtempSync, writeFileSync, rmSync } from 'fs'; -import { join } from 'path'; +import { join, isAbsolute } from 'path'; import { tmpdir } from 'os'; import { loadMounts, @@ -124,7 +124,9 @@ describe('loadMounts — entry validation', () => { mounts: [{ id: 'a', path: '/tmp/relative-test', engine: 'pglite', database_path: '/tmp/a/.pg' }], })); const mounts = loadMounts(path); - expect(mounts[0].path.startsWith('/')).toBe(true); + // loadMounts resolve()s the path: 'C:\…' on win32, so a leading-'/' + // check is the wrong absoluteness test. + expect(isAbsolute(mounts[0].path)).toBe(true); }); test('enabled=false is preserved', () => { diff --git a/test/brain-writer-walk-prune.test.ts b/test/brain-writer-walk-prune.test.ts index 9b455ca25..59555fdf5 100644 --- a/test/brain-writer-walk-prune.test.ts +++ b/test/brain-writer-walk-prune.test.ts @@ -20,11 +20,20 @@ import { describe, expect, test, beforeAll, afterAll } from 'bun:test'; import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'fs'; import { execFileSync } from 'child_process'; -import { join } from 'path'; +import { join, sep } from 'path'; import { tmpdir } from 'os'; import { scanBrainSources, walkDir } from '../src/core/brain-writer.ts'; import { collectFiles } from '../src/commands/frontmatter.ts'; +/** + * Build a separator-prefixed path fragment for suffix/substring predicates. + * The walkers emit native-separator paths, so a hardcoded '/' fragment makes + * the positive assertions fail on Windows AND — worse — makes every negative + * `toBe(false)` guard pass vacuously, proving nothing. Byte-identical to the + * old '/foo/bar' literals on POSIX. + */ +const seg = (...parts: string[]) => sep + join(...parts); + let root: string; beforeAll(() => { @@ -64,19 +73,19 @@ describe('walkDir (brain-writer.ts) — descent-time pruning', () => { test('does NOT descend into node_modules at any depth', () => { const visited: string[] = []; walkDir(root, () => {}, (dir) => visited.push(dir)); - expect(visited.some(d => d.includes('/node_modules'))).toBe(false); + expect(visited.some(d => d.includes(seg('node_modules')))).toBe(false); }); test('does NOT descend into .git', () => { const visited: string[] = []; walkDir(root, () => {}, (dir) => visited.push(dir)); - expect(visited.some(d => d.endsWith('/.git') || d.includes('/.git/'))).toBe(false); + expect(visited.some(d => d.endsWith(seg('.git')) || d.includes(seg('.git') + sep))).toBe(false); }); test('does NOT descend into .obsidian (dot-prefix heuristic)', () => { const visited: string[] = []; walkDir(root, () => {}, (dir) => visited.push(dir)); - expect(visited.some(d => d.includes('/.obsidian'))).toBe(false); + expect(visited.some(d => d.includes(seg('.obsidian')))).toBe(false); }); test('does NOT descend into *.raw sidecar dirs', () => { @@ -88,22 +97,22 @@ describe('walkDir (brain-writer.ts) — descent-time pruning', () => { test('does NOT descend into git submodule directories (.git as FILE)', () => { const visited: string[] = []; walkDir(root, () => {}, (dir) => visited.push(dir)); - expect(visited.some(d => d.endsWith('/people/submod'))).toBe(false); + expect(visited.some(d => d.endsWith(seg('people', 'submod')))).toBe(false); }); test('DOES descend into regular subdirs and visits .md files there', () => { const visited: string[] = []; const files: string[] = []; walkDir(root, (f) => { files.push(f); }, (dir) => visited.push(dir)); - expect(visited.some(d => d.endsWith('/people'))).toBe(true); - expect(visited.some(d => d.endsWith('/concepts/subdir'))).toBe(true); + expect(visited.some(d => d.endsWith(seg('people')))).toBe(true); + expect(visited.some(d => d.endsWith(seg('concepts', 'subdir')))).toBe(true); // ops/ is ordinary content — descended, not pruned (#2404). - expect(visited.some(d => d.endsWith('/ops/logs'))).toBe(true); - expect(files.some(f => f.endsWith('/people/alice.md'))).toBe(true); - expect(files.some(f => f.endsWith('/concepts/subdir/thing.md'))).toBe(true); - expect(files.some(f => f.endsWith('/ops/logs/run.md'))).toBe(true); + expect(visited.some(d => d.endsWith(seg('ops', 'logs')))).toBe(true); + expect(files.some(f => f.endsWith(seg('people', 'alice.md')))).toBe(true); + expect(files.some(f => f.endsWith(seg('concepts', 'subdir', 'thing.md')))).toBe(true); + expect(files.some(f => f.endsWith(seg('ops', 'logs', 'run.md')))).toBe(true); // And explicitly does NOT visit the file under node_modules. - expect(files.some(f => f.includes('/node_modules/'))).toBe(false); + expect(files.some(f => f.includes(seg('node_modules') + sep))).toBe(false); }); test('regression: pre-v0.38.2.0 walker would have descended into node_modules and stat\'d every entry', () => { @@ -112,7 +121,7 @@ describe('walkDir (brain-writer.ts) — descent-time pruning', () => { // visitDir would be called with node_modules paths. const descents: string[] = []; walkDir(root, () => {}, (d) => descents.push(d)); - const vendor = descents.filter(d => /\/(node_modules|\.git|\.obsidian)(\/|$)/.test(d) || /\.raw$/.test(d)); + const vendor = descents.filter(d => /[\\/](node_modules|\.git|\.obsidian)([\\/]|$)/.test(d) || /\.raw$/.test(d)); expect(vendor).toEqual([]); }); }); @@ -121,38 +130,38 @@ describe('collectFiles (frontmatter.ts) — descent-time pruning parity', () => test('does NOT descend into node_modules at any depth', () => { const visited: string[] = []; collectFiles(root, (dir) => visited.push(dir)); - expect(visited.some(d => d.includes('/node_modules'))).toBe(false); + expect(visited.some(d => d.includes(seg('node_modules')))).toBe(false); }); test('does NOT descend into .git, .obsidian, or *.raw', () => { const visited: string[] = []; collectFiles(root, (dir) => visited.push(dir)); - expect(visited.some(d => d.includes('/.git'))).toBe(false); - expect(visited.some(d => d.includes('/.obsidian'))).toBe(false); + expect(visited.some(d => d.includes(seg('.git')))).toBe(false); + expect(visited.some(d => d.includes(seg('.obsidian')))).toBe(false); expect(visited.some(d => d.endsWith('.raw'))).toBe(false); }); test('DOES descend into ops/ — ordinary content, not a vendor tree (#2404)', () => { const visited: string[] = []; collectFiles(root, (dir) => visited.push(dir)); - expect(visited.some(d => d.endsWith('/ops') || d.includes('/ops/'))).toBe(true); + expect(visited.some(d => d.endsWith(seg('ops')) || d.includes(seg('ops') + sep))).toBe(true); const files = collectFiles(root); - expect(files.some(f => f.endsWith('/ops/logs/run.md'))).toBe(true); + expect(files.some(f => f.endsWith(seg('ops', 'logs', 'run.md')))).toBe(true); }); test('does NOT descend into git submodule directories', () => { const visited: string[] = []; collectFiles(root, (dir) => visited.push(dir)); - expect(visited.some(d => d.endsWith('/people/submod'))).toBe(false); + expect(visited.some(d => d.endsWith(seg('people', 'submod')))).toBe(false); }); test('DOES collect .md files under regular subdirs', () => { const files = collectFiles(root); - expect(files.some(f => f.endsWith('/people/alice.md'))).toBe(true); - expect(files.some(f => f.endsWith('/concepts/subdir/thing.md'))).toBe(true); - expect(files.some(f => f.includes('/node_modules/'))).toBe(false); - expect(files.some(f => f.includes('/.git/'))).toBe(false); - expect(files.some(f => f.includes('.raw/'))).toBe(false); + expect(files.some(f => f.endsWith(seg('people', 'alice.md')))).toBe(true); + expect(files.some(f => f.endsWith(seg('concepts', 'subdir', 'thing.md')))).toBe(true); + expect(files.some(f => f.includes(seg('node_modules') + sep))).toBe(false); + expect(files.some(f => f.includes(seg('.git') + sep))).toBe(false); + expect(files.some(f => f.includes('.raw' + sep))).toBe(false); }); test('single-file target returns that file unchanged (no walk)', () => { @@ -173,9 +182,9 @@ describe('frontmatter walkers — git-visible file parity', () => { writeFileSync(join(repo, 'people', 'alice.md'), '---\ntitle: Alice\n---\n\nbody\n'); writeFileSync(join(repo, 'local-skills', 'SKILL.md'), '---\nname: bad\n# malformed frontmatter\n'); - const files = collectFiles(repo).map((f) => f.replace(repo + '/', '')); - expect(files).toContain('people/alice.md'); - expect(files).not.toContain('local-skills/SKILL.md'); + const files = collectFiles(repo).map((f) => f.replace(repo + sep, '')); + expect(files).toContain(join('people', 'alice.md')); + expect(files).not.toContain(join('local-skills', 'SKILL.md')); } finally { rmSync(repo, { recursive: true, force: true }); } diff --git a/test/check-resolvable-cli.test.ts b/test/check-resolvable-cli.test.ts index d8feaab3a..84de232e5 100644 --- a/test/check-resolvable-cli.test.ts +++ b/test/check-resolvable-cli.test.ts @@ -133,7 +133,8 @@ describe('check-resolvable — unit: resolveSkillsDir', () => { it('resolves relative --skills-dir against cwd', () => { const r = resolveSkillsDir({ help: false, json: false, fix: false, dryRun: false, verbose: false, strict: false, skillsDir: 'skills' }); - expect(r.dir).toMatch(/\/skills$/); + // r.dir is join()/resolve()-built, so the separator is '\' on win32. + expect(r.dir).toMatch(/[\\/]skills$/); expect(r.error).toBeNull(); expect(r.source).toBe('explicit'); }); @@ -155,7 +156,8 @@ describe('check-resolvable — unit: resolveSkillsDir', () => { const r = resolveSkillsDir({ help: false, json: false, fix: false, dryRun: false, verbose: false, strict: false, skillsDir: null }); // Install-path fallback succeeds when test runs inside the gbrain repo. expect(r.error).toBeNull(); - expect(r.dir).toMatch(/\/skills$/); + // r.dir is join()/resolve()-built, so the separator is '\' on win32. + expect(r.dir).toMatch(/[\\/]skills$/); expect(r.source).toBe('install_path'); } finally { process.chdir(original); @@ -172,7 +174,8 @@ describe('check-resolvable — unit: resolveSkillsDir', () => { // back-compat. See src/core/repo-root.ts. const r = resolveSkillsDir({ help: false, json: false, fix: false, dryRun: false, verbose: false, strict: false, skillsDir: null }); expect(r.error).toBeNull(); - expect(r.dir).toMatch(/\/skills$/); + // r.dir is join()/resolve()-built, so the separator is '\' on win32. + expect(r.dir).toMatch(/[\\/]skills$/); expect(r.source).toBe('cwd_walk_up'); }); @@ -373,7 +376,8 @@ describe('gbrain check-resolvable CLI — integration', () => { const r = run([]); expect(r.status === 0 || r.status === 1).toBe(true); expect(r.stdout).toContain('Auto-detected skills directory'); - expect(r.stdout).toContain('/skills'); + // The logged path is native-format — '\skills' on win32. + expect(r.stdout).toMatch(/[\\/]skills/); }); // v0.31.7 D6 regression guard: --fix must refuse install-path fallback. diff --git a/test/commands/schema-packpath.test.ts b/test/commands/schema-packpath.test.ts index fffb2aeaa..a54071bb1 100644 --- a/test/commands/schema-packpath.test.ts +++ b/test/commands/schema-packpath.test.ts @@ -24,7 +24,12 @@ describe('schema packPathByName', () => { for (const name of ['gbrain-base', 'gbrain-recommended', 'gbrain-base-v2']) { const path = _testHelpers.packPathByName(name); expect(path).toBeTruthy(); - expect(path!.endsWith(`src/core/schema-pack/base/${name}.yaml`)).toBe(true); + // The resolved path is native-format, so it ends with `src\core\...` + // on Windows. Only the separator comes from `path`; the directory + // structure stays hand-written. Byte-identical on POSIX. + expect(path!.endsWith(join('src', 'core', 'schema-pack', 'base', `${name}.yaml`))).toBe( + true, + ); expect(existsSync(path!)).toBe(true); } }); diff --git a/test/e2e/migration-flow.test.ts b/test/e2e/migration-flow.test.ts index c0726e4be..bfacb7ba1 100644 --- a/test/e2e/migration-flow.test.ts +++ b/test/e2e/migration-flow.test.ts @@ -250,7 +250,8 @@ describeE2E('E2E: v0.11.0 orchestrator against live Postgres', () => { for (const todo of todos) { expect(todo.type).toBe('cron-handler-needs-host-registration'); expect(todo.status).toBe('pending'); - expect(todo.manifest_path).toContain('cron/jobs.json'); + // manifest_path is join(scope, 'cron', 'jobs.json') — '\' on win32. + expect(todo.manifest_path).toContain(join('cron', 'jobs.json')); } } finally { restoreHomePath(); diff --git a/test/e2e/multi-source-bug-class.test.ts b/test/e2e/multi-source-bug-class.test.ts index a039e7da4..a3c12d3fb 100644 --- a/test/e2e/multi-source-bug-class.test.ts +++ b/test/e2e/multi-source-bug-class.test.ts @@ -28,7 +28,7 @@ import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; import { tmpdir } from 'node:os'; import { mkdtempSync, rmSync, existsSync, readFileSync, writeFileSync, mkdirSync } from 'node:fs'; -import { join } from 'node:path'; +import { join, sep } from 'node:path'; import { PGLiteEngine } from '../../src/core/pglite-engine.ts'; import { resetPgliteState } from '../helpers/reset-pglite.ts'; import { validateSourceId } from '../../src/core/utils.ts'; @@ -253,7 +253,8 @@ describe('multi-source bug class', () => { // The two paths must NOT collide. expect(defaultPath).not.toBe(mediaPath); - expect(mediaPath).toContain('.sources/media-corpus/'); + // computePath joins, so the segment separator is '\' on win32. + expect(mediaPath).toContain(join('.sources', 'media-corpus') + sep); // Actually write to both paths to prove disk separation. mkdirSync(join(tmpDir, 'people'), { recursive: true }); diff --git a/test/import-checkpoint.test.ts b/test/import-checkpoint.test.ts index 95208cd96..7fac15fa1 100644 --- a/test/import-checkpoint.test.ts +++ b/test/import-checkpoint.test.ts @@ -262,7 +262,9 @@ describe('resumeFilter', () => { '/tmp/example-brain/meetings/2026-05-13.md', '/tmp/example-brain/concepts/a.md', ]; - const completed = new Set(['meetings/2026-05-13.md']); + // resumeFilter keys on relative(dir, p), which carries native separators — + // build the completed key with join() so it matches on Windows too. + const completed = new Set([join('meetings', '2026-05-13.md')]); expect(resumeFilter(all, '/tmp/example-brain', completed)).toEqual([ '/tmp/example-brain/concepts/a.md', ]); diff --git a/test/import-git-fastpath-prune.test.ts b/test/import-git-fastpath-prune.test.ts index 8a9238215..2604c0b4e 100644 --- a/test/import-git-fastpath-prune.test.ts +++ b/test/import-git-fastpath-prune.test.ts @@ -67,15 +67,19 @@ afterAll(() => { describe('#2607 — git fast path excludes what incremental sync excludes', () => { test('tracked files under pruned dirs are NOT collected', () => { const files = rel(collectSyncableFiles(repo, { strategy: 'markdown' })); - expect(files).toContain('notes/real.md'); - expect(files).toContain('ops/tasks.md'); // ordinary content (#2404) - expect(files).not.toContain('.obsidian/plugin-notes.md'); - expect(files).not.toContain('vendor/pkg/notes.md'); - expect(files).not.toContain('node_modules/dep/CHANGELOG.md'); - expect(files).not.toContain('people/pedro.raw/source.md'); + // rel() returns relative() output, which carries native separators — build + // every expectation with join() to match. A hardcoded '/' literal would not + // just fail the positives, it would make each not.toContain() below pass + // vacuously on Windows, silently retiring the #2607 guard. + expect(files).toContain(join('notes', 'real.md')); + expect(files).toContain(join('ops', 'tasks.md')); // ordinary content (#2404) + expect(files).not.toContain(join('.obsidian', 'plugin-notes.md')); + expect(files).not.toContain(join('vendor', 'pkg', 'notes.md')); + expect(files).not.toContain(join('node_modules', 'dep', 'CHANGELOG.md')); + expect(files).not.toContain(join('people', 'pedro.raw', 'source.md')); // Metafiles stay excluded too. expect(files).not.toContain('README.md'); - expect(files).not.toContain('notes/index.md'); + expect(files).not.toContain(join('notes', 'index.md')); }); test('full-sync enumeration agrees with incremental isSyncable for every collected file', () => { @@ -104,8 +108,8 @@ describe('#2607 — git fast path excludes what incremental sync excludes', () = includeGitignored: true, })); - expect(defaultFiles).not.toContain('Meetings/weekly.md'); - expect(includeIgnored).toContain('Meetings/weekly.md'); + expect(defaultFiles).not.toContain(join('Meetings', 'weekly.md')); + expect(includeIgnored).toContain(join('Meetings', 'weekly.md')); } finally { rmSync(ignoredRepo, { recursive: true, force: true }); } diff --git a/test/integrations.test.ts b/test/integrations.test.ts index 1ac7e8f96..4e2a90f7e 100644 --- a/test/integrations.test.ts +++ b/test/integrations.test.ts @@ -2,6 +2,7 @@ import { describe, test, expect, beforeAll, beforeEach, afterEach } from 'bun:te import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs'; import { tmpdir } from 'os'; import { join } from 'path'; +import { resolve, sep } from 'node:path'; import { parseRecipe, isUnsafeHealthCheck, @@ -14,6 +15,8 @@ import { isInternalUrl, } from '../src/commands/integrations.ts'; +const RECIPES_DIR = resolve(import.meta.dir, '..', 'recipes'); + // --- parseRecipe tests --- describe('parseRecipe', () => { @@ -259,7 +262,7 @@ describe('twilio-voice-brain recipe', () => { ); const recipe = parseRecipe(content, 'twilio-voice-brain.md'); expect(recipe).not.toBeNull(); - const recipesDir = new URL('../recipes/', import.meta.url).pathname; + const recipesDir = RECIPES_DIR; for (const dep of recipe!.frontmatter.requires) { const depPath = resolve(recipesDir, `${dep}.md`); expect(existsSync(depPath)).toBe(true); @@ -302,7 +305,7 @@ describe('all recipes', () => { test('every recipe file in recipes/ parses correctly', () => { const { readFileSync, readdirSync } = require('fs'); const { resolve } = require('path'); - const recipesDir = new URL('../recipes/', import.meta.url).pathname; + const recipesDir = RECIPES_DIR; const files = readdirSync(recipesDir).filter((f: string) => f.endsWith('.md')); expect(files.length).toBeGreaterThan(0); for (const file of files) { @@ -316,7 +319,7 @@ describe('all recipes', () => { test('no recipe contains personal references', () => { const { readFileSync, readdirSync } = require('fs'); const { resolve } = require('path'); - const recipesDir = new URL('../recipes/', import.meta.url).pathname; + const recipesDir = RECIPES_DIR; const files = readdirSync(recipesDir).filter((f: string) => f.endsWith('.md')); const personalPatterns = /wintermute|mercury|16507969501|\+1650796/i; for (const file of files) { @@ -328,7 +331,7 @@ describe('all recipes', () => { test('typed health_checks parse correctly in all recipes', () => { const { readFileSync, readdirSync } = require('fs'); const { resolve } = require('path'); - const recipesDir = new URL('../recipes/', import.meta.url).pathname; + const recipesDir = RECIPES_DIR; const files = readdirSync(recipesDir).filter((f: string) => f.endsWith('.md')); for (const file of files) { const content = readFileSync(resolve(recipesDir, file), 'utf-8'); @@ -665,7 +668,9 @@ describe('getRecipeDirs (B1 trust boundary)', () => { expect(typeof d.dir).toBe('string'); } // In this repo, the source recipes dir must be trusted - const source = dirs.find(d => d.dir.endsWith('/recipes') && d.trusted); + // `dir` is native-format (`...\recipes` on Windows). Only the separator + // comes from `path`; the directory name stays hand-written. + const source = dirs.find(d => d.dir.endsWith(`${sep}recipes`) && d.trusted); expect(source).toBeDefined(); }); diff --git a/test/migrations-v0_11_0.test.ts b/test/migrations-v0_11_0.test.ts index 8af2d9662..0e1a6d176 100644 --- a/test/migrations-v0_11_0.test.ts +++ b/test/migrations-v0_11_0.test.ts @@ -12,7 +12,7 @@ import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; import { mkdtempSync, rmSync, mkdirSync, writeFileSync, readFileSync, existsSync, symlinkSync } from 'fs'; -import { join } from 'path'; +import { join, sep, dirname } from 'path'; import { tmpdir } from 'os'; import { __testing, type PendingHostWorkEntry } from '../src/commands/migrations/v0_11_0.ts'; @@ -55,8 +55,6 @@ function writeCronJson(dir: string, jobs: unknown[]) { writeFileSync(path, JSON.stringify({ jobs }, null, 2) + '\n'); return path; } -// Re-export dirname so writeCronJson can use it without another import -const dirname = (p: string) => p.substring(0, p.lastIndexOf('/')); const DEFAULT_OPTS = { yes: true, @@ -277,13 +275,17 @@ describe('findAgentsMdFiles + findCronManifests scoping', () => { test('does NOT walk $PWD unless --host-dir is passed', () => { mkdirSync(join(tmp, 'project'), { recursive: true }); writeFileSync(join(tmp, 'project', 'AGENTS.md'), '# project\n'); + // findAgentsMdFiles returns join()-built native paths, so a '/project/' + // literal matches nothing on win32 and the negative assertion below would + // pass vacuously. Only the separator comes from `path`; the directory name + // being probed stays hand-written. // No --host-dir const found = findAgentsMdFiles(DEFAULT_OPTS); - expect(found.some(p => p.includes('/project/'))).toBe(false); + expect(found.some(p => p.includes(`${sep}project${sep}`))).toBe(false); // With --host-dir const foundWithHostDir = findAgentsMdFiles({ ...DEFAULT_OPTS, hostDir: join(tmp, 'project') }); - expect(foundWithHostDir.some(p => p.includes('/project/'))).toBe(true); + expect(foundWithHostDir.some(p => p.includes(`${sep}project${sep}`))).toBe(true); }); test('findCronManifests picks up cron/jobs.json under scoped roots', () => { diff --git a/test/mounts-cache.test.ts b/test/mounts-cache.test.ts index 4813563e7..763c1672b 100644 --- a/test/mounts-cache.test.ts +++ b/test/mounts-cache.test.ts @@ -100,7 +100,9 @@ describe('composeResolvers — mount skills', () => { expect(result.entries).toHaveLength(1); expect(result.entries[0].qualifiedName).toBe('yc-media::ingest'); expect(result.entries[0].brainId).toBe('yc-media'); - expect(result.entries[0].absolutePath).toContain('/skills/ingest/SKILL.md'); + // absolutePath is join()-built, so it carries '\' on win32 — build the + // expected tail with join() rather than a POSIX literal. + expect(result.entries[0].absolutePath).toContain(join('/skills', 'ingest', 'SKILL.md')); }); test('disabled mount is excluded', () => { diff --git a/test/mounts-cli.test.ts b/test/mounts-cli.test.ts index 59e3471f3..95ad83aec 100644 --- a/test/mounts-cli.test.ts +++ b/test/mounts-cli.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect, afterEach, beforeEach } from 'bun:test'; import { mkdtempSync, rmSync, mkdirSync, writeFileSync, existsSync } from 'fs'; -import { join } from 'path'; +import { join, isAbsolute } from 'path'; import { tmpdir, homedir } from 'os'; import { __testing } from '../src/commands/mounts.ts'; @@ -52,7 +52,9 @@ describe('parseAddArgs', () => { expect(parsed.id).toBe('yc-media'); expect(parsed.engine).toBe('pglite'); expect(parsed.database_path).toBe('/tmp/yc-media/.pg'); - expect(parsed.path.startsWith('/')).toBe(true); + // parsed.path is resolve()-built: 'C:\…' on win32, so a leading-'/' + // check is the wrong absoluteness test. + expect(isAbsolute(parsed.path)).toBe(true); }); test('minimal postgres add', () => { diff --git a/test/notability-eval.test.ts b/test/notability-eval.test.ts index 2b8bda665..a9d0e12a0 100644 --- a/test/notability-eval.test.ts +++ b/test/notability-eval.test.ts @@ -98,8 +98,10 @@ describe('walkMarkdownFiles', () => { const out = walkMarkdownFiles(root); const sorted = out.slice().sort(); expect(sorted.length).toBe(2); - expect(sorted[0]).toBe('meetings/one.md'); - expect(sorted[1]).toBe('personal/three.md'); + // walkMarkdownFiles builds its relative paths with join(), which emits + // '\' on win32 — build the expectations the same way. + expect(sorted[0]).toBe(join('meetings', 'one.md')); + expect(sorted[1]).toBe(join('personal', 'three.md')); } finally { rmSync(root, { recursive: true, force: true }); } @@ -180,8 +182,12 @@ describe('JSONL utilities', () => { }); test('default paths resolve under ~/.gbrain/eval/', () => { - expect(defaultMiningOutPath()).toContain('.gbrain/eval/notability-mining-candidates.jsonl'); - expect(defaultReviewOutPath()).toContain('.gbrain/eval/notability-real.jsonl'); + // Both defaults are join()-built, so the substring must carry native + // separators too ('\.gbrain\eval\…' on win32). + expect(defaultMiningOutPath()).toContain( + join('.gbrain', 'eval', 'notability-mining-candidates.jsonl'), + ); + expect(defaultReviewOutPath()).toContain(join('.gbrain', 'eval', 'notability-real.jsonl')); }); }); diff --git a/test/skill-catalog.test.ts b/test/skill-catalog.test.ts index a8a118150..1670543f8 100644 --- a/test/skill-catalog.test.ts +++ b/test/skill-catalog.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from 'bun:test'; -import { join } from 'path'; +import { join, sep } from 'path'; import type { OperationContext } from '../src/core/operations.ts'; import { buildSkillCatalog, @@ -158,6 +158,6 @@ describe('crossReferenceTools', () => { describe('resolveSkillMdPath (happy path)', () => { test('resolves a real skill to its SKILL.md', () => { const p = resolveSkillMdPath(FIXTURE, 'brain-ops'); - expect(p.endsWith('/brain-ops/SKILL.md')).toBe(true); + expect(p.endsWith(sep + join('brain-ops', 'SKILL.md'))).toBe(true); }); }); diff --git a/test/skillpack-bootstrap-display.test.ts b/test/skillpack-bootstrap-display.test.ts index 57107c53a..1b704e46f 100644 --- a/test/skillpack-bootstrap-display.test.ts +++ b/test/skillpack-bootstrap-display.test.ts @@ -54,7 +54,8 @@ describe('buildBootstrapDisplay', () => { workspace: '/ws', }); expect(r.shown).toBe(false); - expect(r.bootstrapPath).toContain('runbooks/bootstrap.md'); + // bootstrapPath is join(packRoot, relPath), so it carries '\' on win32. + expect(r.bootstrapPath).toContain(join('runbooks', 'bootstrap.md')); }); test('returns shown=false when bootstrap.md is empty (whitespace only)', () => { diff --git a/test/skillpack-copy.test.ts b/test/skillpack-copy.test.ts index 67f8f1beb..07d19620e 100644 --- a/test/skillpack-copy.test.ts +++ b/test/skillpack-copy.test.ts @@ -41,7 +41,12 @@ describe('walkSourceDir', () => { const items = walkSourceDir(src, '/some/dst'); expect(items).toHaveLength(2); - expect(items.map(i => i.target).sort()).toEqual(['/some/dst/a.txt', '/some/dst/b.txt']); + // Build expectations with join(), the same way walkSourceDir builds targets, + // so they carry native separators (byte-identical on POSIX). + expect(items.map(i => i.target).sort()).toEqual([ + join('/some/dst', 'a.txt'), + join('/some/dst', 'b.txt'), + ]); }); it('walks nested directories recursively, mirroring structure', () => { @@ -54,7 +59,11 @@ describe('walkSourceDir', () => { const items = walkSourceDir(src, '/dst'); expect(items).toHaveLength(3); const targets = items.map(i => i.target).sort(); - expect(targets).toEqual(['/dst/sub/deeper/low.txt', '/dst/sub/mid.txt', '/dst/top.txt']); + expect(targets).toEqual([ + join('/dst', 'sub', 'deeper', 'low.txt'), + join('/dst', 'sub', 'mid.txt'), + join('/dst', 'top.txt'), + ]); }); it('returns empty array for a non-existent source directory', () => { @@ -215,6 +224,38 @@ describe('copyArtifacts — canonical-path containment (harvest path)', () => { const dst = scratch('copy-dst-'); const result = copyArtifacts(walkSourceDir(skillDir, dst), { confineRealpath: skillDir }); expect(result.summary.wroteNew).toBe(1); + expect(readFileSync(join(dst, 'SKILL.md'), 'utf-8')).toBe('safe'); + }); + + // Both sides of the containment check are realpathSync() output, so the + // prefix separator has to be the native one. A hardcoded '/' never matches + // a win32 realpath, which rejected EVERY source as path_traversal and made + // harvest a dead feature there. These two pin the fix from both directions: + // an in-root source is accepted, and the sibling-prefix guard the separator + // exists to provide still holds. Asserting only the first would pass equally + // well with the containment check deleted. + it('containment boundary holds with native separators (foo does not match foobar)', () => { + const harvestRoot = scratch('copy-harvest-'); + const skillDir = join(harvestRoot, 'skills', 'foo'); + const siblingDir = join(harvestRoot, 'skills', 'foobar'); + mkdirSync(skillDir, { recursive: true }); + mkdirSync(siblingDir, { recursive: true }); + writeFileSync(join(siblingDir, 'SKILL.md'), 'sibling'); + + const dst = scratch('copy-dst-'); + // Sources live in `skills/foobar`, confinement root is `skills/foo`. + // Rejected only because the prefix carries a trailing separator. + const items = walkSourceDir(siblingDir, dst); + + try { + copyArtifacts(items, { confineRealpath: skillDir }); + throw new Error('expected copyArtifacts to reject the sibling-prefix source'); + } catch (err) { + expect(err).toBeInstanceOf(CopyError); + expect((err as CopyError).code).toBe('path_traversal'); + } + + expect(existsSync(join(dst, 'SKILL.md'))).toBe(false); }); }); diff --git a/test/skillpack-init-pack.test.ts b/test/skillpack-init-pack.test.ts index 89e6a66f2..35c828da4 100644 --- a/test/skillpack-init-pack.test.ts +++ b/test/skillpack-init-pack.test.ts @@ -74,7 +74,10 @@ describe('runInitScaffold — cathedral default', () => { writeFileSync(join(dir, 'skills/preexist/SKILL.md'), 'user content'); const result = runInitScaffold({ targetDir: dir, name: 'preexist' }); // The user's SKILL.md should be in filesSkippedExisting - expect(result.filesSkippedExisting.some((p) => p.endsWith('skills/preexist/SKILL.md'))).toBe(true); + // filesSkippedExisting holds join()-built paths — '\' on win32. + expect( + result.filesSkippedExisting.some((p) => p.endsWith(join('skills', 'preexist', 'SKILL.md'))), + ).toBe(true); // And contents preserved. expect(require('fs').readFileSync(join(dir, 'skills/preexist/SKILL.md'), 'utf-8')).toBe('user content'); }); diff --git a/test/skillpack-install.test.ts b/test/skillpack-install.test.ts index 1420357ca..9bd8c2f12 100644 --- a/test/skillpack-install.test.ts +++ b/test/skillpack-install.test.ts @@ -14,7 +14,7 @@ import { utimesSync, writeFileSync, } from 'fs'; -import { dirname, join } from 'path'; +import { dirname, join, sep } from 'path'; import { tmpdir } from 'os'; import { @@ -169,13 +169,16 @@ describe('enumerateBundle (D-CX-10 dependency closure)', () => { const m = loadBundleManifest(gbrainRoot); const entries = enumerateBundle({ gbrainRoot, skillSlug: 'alpha', manifest: m }); const targets = entries.map(e => e.relTarget).sort(); - expect(targets).toContain('alpha/SKILL.md'); - expect(targets).toContain('alpha/scripts/alpha.mjs'); + // relTarget carries native separators — build expectations with join()/sep + // so the positives match and the `beta` exclusion below stays meaningful + // instead of passing vacuously on Windows. + expect(targets).toContain(join('alpha', 'SKILL.md')); + expect(targets).toContain(join('alpha', 'scripts', 'alpha.mjs')); // Shared deps pulled in despite single-skill scope. - expect(targets).toContain('conventions/quality.md'); + expect(targets).toContain(join('conventions', 'quality.md')); expect(targets).toContain('_output-rules.md'); // beta NOT included. - expect(targets.find(t => t.startsWith('beta/'))).toBeUndefined(); + expect(targets.find(t => t.startsWith('beta' + sep))).toBeUndefined(); }); it('throws BundleError for unknown skill slug', () => { const { gbrainRoot } = scratchGbrain(); @@ -189,8 +192,8 @@ describe('enumerateBundle (D-CX-10 dependency closure)', () => { const m = loadBundleManifest(gbrainRoot); const entries = enumerateBundle({ gbrainRoot, manifest: m }); const targets = entries.map(e => e.relTarget).sort(); - expect(targets.some(t => t.startsWith('alpha/'))).toBe(true); - expect(targets.some(t => t.startsWith('beta/'))).toBe(true); + expect(targets.some(t => t.startsWith('alpha' + sep))).toBe(true); + expect(targets.some(t => t.startsWith('beta' + sep))).toBe(true); }); }); diff --git a/test/sources-mcp.test.ts b/test/sources-mcp.test.ts index 65991121e..f042a3ae4 100644 --- a/test/sources-mcp.test.ts +++ b/test/sources-mcp.test.ts @@ -274,7 +274,8 @@ describe('sources_add — remote callers ignore path/clone_dir overrides', () => })) as any; // Clone landed at the SAFE default, not /etc/gbrain-pwned. expect(row.local_path).not.toBe('/etc/gbrain-pwned'); - expect(row.local_path).toContain('clones/attack-clone-dir'); + // defaultCloneDir → gbrainPath('clones', id) → join, so '\' on win32. + expect(row.local_path).toContain(join('clones', 'attack-clone-dir')); // /etc/gbrain-pwned was never written. expect(existsSync('/etc/gbrain-pwned')).toBe(false); }); diff --git a/test/sync-walker-symlink.test.ts b/test/sync-walker-symlink.test.ts index 1f3ed3432..023c18265 100644 --- a/test/sync-walker-symlink.test.ts +++ b/test/sync-walker-symlink.test.ts @@ -16,13 +16,30 @@ */ import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync, rmSync } from 'fs'; -import { join } from 'path'; +import { basename, join, sep } from 'path'; import { tmpdir } from 'os'; import { collectSyncableFiles } from '../src/commands/import.ts'; import { withEnv } from './helpers/with-env.ts'; +/** + * PLATFORM NOTE: collectSyncableFiles returns absolute paths built with + * join(), so on win32 they carry '\' — correct, since callers feed them to + * readFileSync. Two POSIX-only idioms silently no-op there: `split('/').pop()` + * returns the WHOLE path instead of a basename, and `replace(tmp, '')` leaves + * a '\'-prefixed tail. Both turn the `startsWith('/.git')` style negative + * assertions below into vacuous passes, so the skip-list regressions this file + * exists to pin would stop being tested at all. + * + * Expectations therefore stay NATIVE: only the separator comes from `path`, + * the directory structure in each one stays hand-written. Byte-identical to + * the old POSIX literals when sep is '/'. + */ + let tmp: string; +/** Path relative to `tmp`, native separators, leading separator preserved. */ +const relToTmp = (p: string): string => p.slice(tmp.length); + beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), 'gbrain-walker-')); }); @@ -44,7 +61,7 @@ describe('collectSyncableFiles symlink + cycle hardening', () => { expect(ms).toBeLessThan(1000); // would hang if walker followed the loop expect(files).toContain(join(tmp, 'notes.md')); - expect(files.every(f => !f.includes('/loop/'))).toBe(true); + expect(files.every(f => !f.includes(`${sep}loop${sep}`))).toBe(true); }); }); @@ -82,7 +99,7 @@ describe('collectSyncableFiles symlink + cycle hardening', () => { const files = collectSyncableFiles(tmp, { strategy: 'markdown' }); expect(files).toContain(join(tmp, 'shallow.md')); - expect(files.every(f => !f.includes('/d35/'))).toBe(true); // past depth 32 + expect(files.every(f => !f.includes(`${sep}d35${sep}`))).toBe(true); // past depth 32 }); }); @@ -96,9 +113,9 @@ describe('collectSyncableFiles symlink + cycle hardening', () => { const markdown = collectSyncableFiles(tmp, { strategy: 'markdown' }); const auto = collectSyncableFiles(tmp, { strategy: 'auto' }); - expect(code.map(f => f.split('/').pop()).sort()).toEqual(['bar.py', 'foo.ts']); - expect(markdown.map(f => f.split('/').pop())).toEqual(['notes.md']); - expect(auto.map(f => f.split('/').pop()).sort()).toEqual(['bar.py', 'foo.ts', 'notes.md']); + expect(code.map(f => basename(f)).sort()).toEqual(['bar.py', 'foo.ts']); + expect(markdown.map(f => basename(f))).toEqual(['notes.md']); + expect(auto.map(f => basename(f)).sort()).toEqual(['bar.py', 'foo.ts', 'notes.md']); }); }); @@ -113,12 +130,12 @@ describe('collectSyncableFiles symlink + cycle hardening', () => { writeFileSync(join(tmp, 'node_modules/foo/index.md'), 'no\n'); const files = collectSyncableFiles(tmp, { strategy: 'markdown' }); - const names = files.map(f => f.replace(tmp, '')); + const names = files.map(f => relToTmp(f)); - expect(names).toContain('/real.md'); - expect(names.every(n => !n.startsWith('/.git'))).toBe(true); - expect(names.every(n => !n.startsWith('/.claude'))).toBe(true); - expect(names.every(n => !n.startsWith('/node_modules'))).toBe(true); + expect(names).toContain(`${sep}real.md`); + expect(names.every(n => !n.startsWith(`${sep}.git`))).toBe(true); + expect(names.every(n => !n.startsWith(`${sep}.claude`))).toBe(true); + expect(names.every(n => !n.startsWith(`${sep}node_modules`))).toBe(true); }); }); @@ -130,14 +147,14 @@ describe('collectSyncableFiles symlink + cycle hardening', () => { // Off → markdown only. await withEnv({ GBRAIN_EMBEDDING_MULTIMODAL: undefined }, () => { const off = collectSyncableFiles(tmp, { strategy: 'markdown' }); - expect(off.map(f => f.split('/').pop()).sort()).toEqual(['r.md']); + expect(off.map(f => basename(f)).sort()).toEqual(['r.md']); }); // On → markdown + images (preserves v0.27.1 F2 collectMarkdownFiles // behavior; codex C5 carve-out). await withEnv({ GBRAIN_EMBEDDING_MULTIMODAL: 'true' }, () => { const on = collectSyncableFiles(tmp, { strategy: 'markdown' }); - expect(on.map(f => f.split('/').pop()).sort()).toEqual(['j.jpg', 'p.png', 'r.md']); + expect(on.map(f => basename(f)).sort()).toEqual(['j.jpg', 'p.png', 'r.md']); }); }); @@ -156,8 +173,8 @@ describe('collectSyncableFiles symlink + cycle hardening', () => { expect(first).toEqual(second); // Sorted: a.md, b.md, sub/c.md (lexicographic on absolute paths). - expect(first.map(f => f.replace(tmp, ''))).toEqual([ - '/a.md', '/b.md', '/sub/c.md', + expect(first.map(f => relToTmp(f))).toEqual([ + `${sep}a.md`, `${sep}b.md`, `${sep}sub${sep}c.md`, ]); }); });