Repository navigation
Kept every warning in compact sync output and rendered skill symlinks that stay inside their source - #59
Conversation
…ut every other one
There was a problem hiding this comment.
🟡 Changes recommended
The unsynced-directory warning remains excluded from compact output despite the new “every warning” contract.
1 open finding
What changed in this PR
Standardizes warning output and safely materializes in-source skill symlinks across adapters.
Changes:
- Preserves standardized
WARNING:diagnostics in compact and cached output. - Materializes contained skill symlinks while excluding unsafe links.
- Adds documentation, decision records, and regression coverage.
| File | Description |
|---|---|
packages/sync/references/conventions.md |
Documents skill symlink behavior. |
packages/sync/references/adapters.md |
Updates adapter contracts and warning guidance. |
intelligence/rules/shell.md |
Defines the warning format. |
intelligence/rules/engine.md |
Defines engine symlink handling. |
engine/sync.sh |
Inventories skills before linting. |
engine/lib/common.sh |
Implements symlink inventory and materialization. |
engine/adapters/opencode.sh |
Filters commands through the inventory. |
engine/adapters/cursor.sh |
Filters rendered Cursor skills. |
engine/adapters/copilot.sh |
Filters skills and prunes stale links. |
engine/adapters/claude.sh |
Filters skills and prunes stale links. |
docs/cli.md |
Documents compact warning output. |
decisions/0017-render-symlinks-that-stay-inside-their-source.md |
Records the symlink policy. |
cli/tests/unit-engine.sh |
Tests inventory and link resolution. |
cli/tests/e2e-sync-cache.sh |
Tests warning replay and cache behavior. |
cli/tests/e2e-lifecycle.sh |
Covers symlinks across generated trees. |
cli/tests/e2e-compat.sh |
Covers compact frontmatter warnings. |
cli/lib/sync-cache.sh |
Replays standardized warnings. |
cli/lib/registry.sh |
Standardizes registry warnings. |
cli/internal/package-update.sh |
Standardizes update warnings. |
cli/internal/package-search.sh |
Standardizes search warnings. |
cli/internal/convert-legacy.sh |
Standardizes conversion warnings. |
cli/commands/source.sh |
Standardizes missing-source warnings. |
cli/commands/registry.sh |
Standardizes registry-add warnings. |
CHANGELOG.md |
Records the behavior changes. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Verification - BLOCKED (WORKSPACE)head: 82dba06 - steps: PR body "Verification" section, steps 1-3; qa_checks: none configured (no The worktree given for this run ( For the record, not as a verified result: steps were driven against a
Step 3's observed gap traces to No label added by this run. |
Complete - MANUALhead: 82dba06 - not merged; v0.19.0 shipped without this PR by owner decision
|
Verification - PASShead: a231ee9 - steps: sha256:f55a2370e2d32483 - env: local (WSL, Bash 5.2.21, symlink support) Digest: first 16 hex characters of SHA-256 over UTF-8 text strictly between "## Verification\n" and "\n## Automated coverage", preserving section whitespace. Standing checks: none configured; docs/qa-checks.md does not exist. The given workspace was clean and at the named head before and after execution. Ran the declared bash cli/tests/verify.sh all from that workspace using its built-in WSL copy bridge, then ran the PR's exact shell script from a disposable export made with git archive a231ee9. This export is test scratch, with no checkout, ref or workspace mutation. The script's cleanup additionally removes the runtime export. No test accounts needed.
Negative probes implied by passing steps, executed after the declared script: changing the description to exactly 1024 characters and running forced compact sync returned 0 with no "REJECTED at load time" warning. Removing the escaping link and running compact sync returned 0 with no stale "skill 'escape'" warning and no escape output in any of the three trees. FailuresNone. |
Code review - CLEANhead: a231ee9 - base: main (f9a44ec) - files 26, +891/-123 CriticalNone. WarningNone. SuggestionNone. Reviewed the complete base-to-head diff and surrounding consumers against the project profile, shell/CLI/engine/documentation rules, the authoring rules and decisions 0001, 0002 and 0017. The PR body explains the problem and solution, declares the warning-prefix changes outside sync and the cache behavior, and its claimed behavior is delivered by the diff. No migration, environment or secret change requires deployment notes. The shared inventory governs the skill index, lint, directory copies and glob-based adapters; materialized copies receive quoting, effort conversion and Codex invocation-policy enforcement. The regression coverage includes an escaping directory without SKILL.md, nested links, cycles, fail-closed resolver failures, compact warning replay and the byte-identical authored policy. The existing Verification - PASS report at this head is consistent with the code reviewed; this stage did not rerun behavioral QA. Diff secret scan: no live credentials found. Matches were token-expansion prose and the explicit TOP-SECRET-HOST-CONTENT test fixture. The supplied workspace remained clean and at the named head throughout review. PR head and base were rechecked before recording. |
Verification - PASShead: f33d1a7 - steps: sha256:f55a2370e2d32483 - env: local (WSL, Bash 5.2.21, symlink support) Digest: first 16 hex characters of SHA-256 over UTF-8 text strictly between "## Verification\n" and "\n## Automated coverage", preserving section whitespace. Standing checks: none configured; docs/qa-checks.md does not exist. This is a fresh execution against the new head, including the complete all gate, then the current declared script and negative probes. The given workspace was clean and at the named head before and after execution. Ran the declared bash cli/tests/verify.sh all from that workspace using its built-in WSL copy bridge, then ran the PR's exact shell script from a disposable export made with git archive f33d1a7. This export is test scratch, with no checkout, ref or workspace mutation. The script's cleanup additionally removes the runtime export. No test accounts needed.
Negative probes implied by passing steps, executed after the declared script: changing the description to exactly 1024 characters and running forced compact sync returned 0 with no "REJECTED at load time" warning. Removing the escaping link and running compact sync returned 0 with no stale "skill 'escape'" warning and no escape output in any of the three trees. FailuresNone. |
Code review - CLEANhead: f33d1a7 - base: main (f9a44ec) - files 26, +907/-133 CriticalNone. WarningNone. SuggestionNone. Reviewed the current base-to-head change, current PR claims and comments against the project profile, shell/CLI/engine/documentation and authoring rules, and decisions 0001, 0002 and 0017. The warning and skill-link behavior remains consistent across the inventory, adapters, transforms, invocation policies and indexes. No migration, environment or secret change requires deployment notes. The new unsynced-directory diagnostics now use WARNING: lines, including the remediation hint. Both the compact renderer filter and unchanged-run report preserve those lines; the cache regression checks compact output before and after a forced render. Removing the obsolete report prefixes therefore does not drop the diagnostics. This completes the PR body's claim that compact sync retains every warning. The fresh Verification - PASS report for this head is consistent with the reviewed code and coverage. This stage did not rerun behavioral QA. Diff secret scan found no live credentials; matches were token-expansion prose and the explicit TOP-SECRET-HOST-CONTENT fixture. The supplied workspace remained clean at the named head. PR head and base were rechecked before recording. |

Summary
Closes #44, #47, #51 and #52. Sync dropped actionable warnings and handled symlinks in skills sources four different ways:
sync --compactkeeps onlyWARNING:/CONTEXT:lines, but the frontmatter lint (including "will be REJECTED at load time"), the symlink notes and six CLI warnings printed indentedWARN:lines. Every warning is now aWARNING:line, as Added a tool-neutral effort field and stopped deriving Codex reasoning effort from the tier #46 and Warned when an agent tier resolves to no model for a target #49 already do; the unchanged-run replay keeps exactly those. The rule is stated once inintelligence/rules/shell.md.WARNING:naming the skill and the link itself.skill_source_inventorysettles every link once per run, fail-closed, resolving containment withcd -P(noreadlink -f). Claude, Cursor and Copilot no longer receive raw links.own-policycase now pins its keptagents/openai.yamlbyte for byte.Rules and agents sources are not covered by this rule yet; that gap is #58, left out of 0.20.0 by the owner.
Type of change
Public CLI check
intelligence sync(unchanged)Verification
Run on Linux, macOS or WSL with symlink support. From an isolated checkout of this PR head, run
bash cli/tests/verify.sh alland requireverify ok. For the observable cases below, run this script from that checkout; it creates a disposable project and leaves the checkout untouched.Automated coverage
e2e-compat: the 1100-character description warning undersync --compact --force, on the next unchanged compact run, and in the normal replay; fails if the line goes back toWARN:.e2e-lifecycle: in-source symlinked skill directory andSKILL.mdrendered as regular files in every tree; escaping, dangling and looping links left out with theWARNING:; no link in any generated skill tree; the A symlinked SKILL.md keeps its effort as written in every skill tree #47 reproduction (ultra→ Claudemax,.agentsultra, no key in Copilot/Cursor);own-policybyte-identical (fails when the skip inenforce_authored_invocation_policiesis broken).bash cli/tests/verify.sh all; requireverify ok. The symlink lifecycle regression includes an escaping directory whose target has noSKILL.md.Versioned artifacts
0.20.0is pending on main; this adds its lines to## [0.20.0]CHANGELOG.mdrecords the change under## [0.20.0]; published0.19.0stays byte-identicalNotes for reviewers
Behaviour changes worth noting: a skill whose
SKILL.mdlinks outside its source drops out of theAGENTS.mdindex with a warning; the six CLI warnings outside sync changed only theirWARNING:prefix; ine2e-sync-cache, removing an escaping link is now a cache hit, because the link never reached any output.