Skip to content

Kept every warning in compact sync output and rendered skill symlinks that stay inside their source - #59

Merged
dzykovic merged 10 commits into
mainfrom
fix/sync-diagnostics
Oct 7, 2026
Merged

dzykovic merged 10 commits into
mainfrom
fix/sync-diagnostics

Conversation

@dzykovic

@dzykovic dzykovic commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #44, #47, #51 and #52. Sync dropped actionable warnings and handled symlinks in skills sources four different ways:

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

  • CLI lifecycle or package behavior
  • Sync engine or adapter
  • Rule, agent, or skill content
  • Documentation
  • Refactor with no behavior change

Public CLI check

  • Uses only the established public command groups (no command changes)
  • Keeps preview/apply behavior explicit for planned writes (unchanged)
  • Covers automatic project alignment and CI refusal through the existing suites
  • Keeps fresh-clone package restoration behind 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 all and require verify ok. For the observable cases below, run this script from that checkout; it creates a disposable project and leaves the checkout untouched.

set -euo pipefail
CLI="$(pwd)/cli/intelligence"
scratch="$(mktemp -d)"
trap 'rm -rf "$scratch"' EXIT
git init -q "$scratch/project"
cd "$scratch/project"
bash "$CLI" init --targets claude,codex,copilot --bare
mkdir -p intelligence/agents intelligence/skills/demo
{
    printf -- '---\nname: long\ndescription: "'
    printf '%1100s' '' | tr ' ' x
    printf '"\ntier: standard\n---\n\n# Long\n'
} > intelligence/agents/long.md
bash "$CLI" sync --compact --force > first.log 2>&1
grep '^WARNING:.*REJECTED at load time' first.log
bash "$CLI" sync --compact > replay.log 2>&1
grep '^WARNING:.*REJECTED at load time' replay.log
printf -- '---\nname: demo\ndescription: "Demo"\neffort: ultra\n---\n\n# Demo\n' > intelligence/skills/demo/real.md
ln -s real.md intelligence/skills/demo/SKILL.md
bash "$CLI" sync
test -f .claude/skills/demo/SKILL.md && test ! -L .claude/skills/demo/SKILL.md
grep -Fx 'effort: max' .claude/skills/demo/SKILL.md
test -f .github/skills/demo/SKILL.md && test ! -L .github/skills/demo/SKILL.md
if grep -q '^effort:' .github/skills/demo/SKILL.md; then exit 1; fi
test -z "$(find .claude .github .agents -type l)"
mkdir "$scratch/outside-empty"
ln -s "$scratch/outside-empty" intelligence/skills/escape
bash "$CLI" sync --compact > escape.log 2>&1
grep -F "WARNING: skill 'escape' is left out of every output: intelligence/skills/escape is a symlink that resolves outside intelligence/skills" escape.log
for tree in .claude/skills .github/skills .agents/skills; do test ! -e "$tree/escape"; done
bash "$CLI" status --check

Automated coverage

  • e2e-compat: the 1100-character description warning under sync --compact --force, on the next unchanged compact run, and in the normal replay; fails if the line goes back to WARN:.
  • e2e-lifecycle: in-source symlinked skill directory and SKILL.md rendered as regular files in every tree; escaping, dangling and looping links left out with the WARNING:; no link in any generated skill tree; the A symlinked SKILL.md keeps its effort as written in every skill tree #47 reproduction (ultra → Claude max, .agents ultra, no key in Copilot/Cursor); own-policy byte-identical (fails when the skip in enforce_authored_invocation_policies is broken).
  • Run bash cli/tests/verify.sh all; require verify ok. The symlink lifecycle regression includes an escaping directory whose target has no SKILL.md.

Versioned artifacts

  • 0.20.0 is pending on main; this adds its lines to ## [0.20.0]
  • No version file moves
  • CHANGELOG.md records the change under ## [0.20.0]; published 0.19.0 stays byte-identical
  • No stable release is published by this PR

Notes for reviewers

Behaviour changes worth noting: a skill whose SKILL.md links outside its source drops out of the AGENTS.md index with a warning; the six CLI warnings outside sync changed only their WARNING: prefix; in e2e-sync-cache, removing an escaping link is now a cache hit, because the link never reached any output.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 15:36
@dzykovic dzykovic added the ai:processing An agent is working on this pull request label Oct 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Comment thread docs/cli.md
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Verification - BLOCKED (WORKSPACE)

head: 82dba06 - steps: PR body "Verification" section, steps 1-3; qa_checks: none configured (no docs/qa-checks.md) - env: local (runtime exported with git archive <sha> cli engine packages, executed under WSL)

The worktree given for this run (intelligence-sync-diagnostics) was confirmed clean and at this head twice at the start of the run. Partway through step execution, its index picked up roughly thirty staged changes with no overlap with this PR's file list (e.g. decisions/0019-name-package-sources-by-package.md, cli/internal/package-alias.sh), while HEAD itself stayed at this SHA (git reflog still shows 82dba06 as the last entry, committed 18:43:47, no later move). The index's own mtime (18:58:52) falls after the two clean checks (~18:51-18:52) and the runtime archive, so the location moved under this run while it was still in progress. Per protocol this stage writes no factor and holds no per-step verdict for this report — nothing read from this worktree after that point is known to belong to one commit, whatever HEAD still names. This stage ran no checkout/reset/fetch/commit against it and leaves it exactly as found.

For the record, not as a verified result: steps were driven against a cli engine packages tree exported with git archive 82dba063e6773453d23c89b972f0a618de27f029 (independent of the worktree's live files) inside a disposable WSL scratch directory, before the move above was discovered.

# Step Source Observed (unverified - see above)
1 sync --compact --force then sync --compact after a 1100-char agent description PR Both runs: exit 0, WARNING: ... description is 1100 chars ... will be REJECTED at load time printed
2 In-source symlinked SKILL.md (effort: ultra), run sync PR Exit 0; .claude/skills/demo/SKILL.md regular file with effort: max; .github/skills/demo/SKILL.md regular file with no effort: line; no symlinks under .claude/.github/.agents
3 ln -s /etc intelligence/skills/escape, run sync PR Exit 0; no escape directory in any output tree; but no WARNING: line named escape - the step declares one should appear

Step 3's observed gap traces to engine/lib/common.sh _ski_skill (outside:d case): the escaping-directory warning only fires when <target>/SKILL.md exists as a regular file, so a directory escape with no SKILL.md at the resolved target (/etc, as the step specifies) renders silently with no warning, while decision 0017 states every escaping link is "said so" unconditionally. This needs re-running once the workspace is confirmed stable again.

No label added by this run.

@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Complete - MANUAL

head: 82dba06 - not merged; v0.19.0 shipped without this PR by owner decision

  • The finalize run was stopped when main moved (Let sources name a package's directories by full name or alias #48); this PR is not in v0.19.0 and now targets the next version.
  • Defect to fix before merge (found by verification step 3): an escaping skill-directory symlink whose target holds no SKILL.md is left out without the WARNING: decision 0017 requires — _ski_skill, outside:d branch in engine/lib/common.sh warns only when the target has a SKILL.md; e2e-lifecycle only covers a target with one.
  • Next: merge main (conflict in engine/sync.sh: keep Let sources name a package's directories by full name or alias #48's unresolved-reference WARNING: loop, drop the old lint block this PR moved after the inventory), move the CHANGELOG lines to the next version section, fix the defect above with a test, then finalize again.

@dzykovic dzykovic added ai:manual Needs an owner decision and removed ai:processing An agent is working on this pull request labels Oct 7, 2026
@dzykovic dzykovic added ai:processing An agent is working on this pull request and removed ai:manual Needs an owner decision labels Oct 7, 2026
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Verification - PASS

head: 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.

# Step Source Verdict Expected and observed
1 Run bash cli/tests/verify.sh all and require verify ok. PR pass Exit 0; final output "ok: tests: CLI suites" followed by "=== verify ok ===". Both lint gates and all 19 suites passed. WSL reported the Windows-junction probe as non-Windows and the pid-1 permissions probe as not applicable; symlink cases ran.
2 Create disposable Git project; bash "$CLI" init --targets claude,codex,copilot --bare; create the 1100-character long.md. PR script pass Exit 0; "initialized: intelligence.yaml (targets: agents claude codex copilot)", then "IS_STATUS=ok IS_DETAIL=synced=4".
3 sync --compact --force > first.log 2>&1; grep '^WARNING:.*REJECTED at load time' first.log. PR script pass Exit 0; "WARNING: /tmp/tmp.aC8NogBRbZ/project/intelligence/agents/long.md:3 description is 1100 chars — over the 1024-char limit; the skill/agent will be REJECTED at load time".
4 sync --compact > replay.log 2>&1; repeat warning grep. PR script pass Exit 0; the exact same WARNING: line appeared on the next unchanged compact run.
5 Create demo/real.md with effort: ultra, symlink demo/SKILL.md to it, then sync. PR script pass Exit 0; "IS_STATUS=ok IS_DETAIL=synced=4". Claude, Codex and Copilot each printed "skill: demo".
6 Require Claude SKILL.md to be a regular file with effort: max. PR script pass Both file predicates returned 0; grep -Fx printed "effort: max".
7 Require Copilot SKILL.md to be regular without effort:; require no links under .claude .github .agents. PR script pass Both file predicates returned 0; effort grep returned no match; find .claude .github .agents -type l returned empty, and emptiness assertion passed.
8 Create empty outside directory, link intelligence/skills/escape to it, sync --compact, require specified warning and no escape output. PR script pass Exit 0; exact output "WARNING: skill 'escape' is left out of every output: intelligence/skills/escape is a symlink that resolves outside intelligence/skills". All three absence assertions passed for .claude/skills, .github/skills, .agents/skills.
9 bash "$CLI" status --check. PR script pass Exit 0; engine 0.20.0, matching schema and all four contracts checked, ending "all good.".

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.

Failures

None.

@dzykovic dzykovic added the ai:verified Declared behavior verified on the reported head label Oct 7, 2026
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Code review - CLEAN

head: a231ee9 - base: main (f9a44ec) - files 26, +891/-123

Critical

None.

Warning

None.

Suggestion

None.

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.

@dzykovic dzykovic added the ai:reviewed Diff reviewed on the reported head label Oct 7, 2026
@dzykovic dzykovic removed ai:verified Declared behavior verified on the reported head ai:reviewed Diff reviewed on the reported head labels Oct 7, 2026
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Verification - PASS

head: 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.

# Step Source Verdict Expected and observed
1 Run bash cli/tests/verify.sh all and require verify ok. PR pass Exit 0; final output "ok: tests: CLI suites" followed by "=== verify ok ===". Both lint gates and all 19 suites passed; sync-cache reported 270 checks passed. WSL reported the Windows-junction probe as non-Windows and the pid-1 permissions probe as not applicable; symlink cases ran.
2 Create disposable Git project; bash "$CLI" init --targets claude,codex,copilot --bare; create the 1100-character long.md. PR script pass Exit 0; "initialized: intelligence.yaml (targets: agents claude codex copilot)", then "IS_STATUS=ok IS_DETAIL=synced=4".
3 sync --compact --force > first.log 2>&1; grep '^WARNING:.*REJECTED at load time' first.log. PR script pass Exit 0; "WARNING: /tmp/tmp.c9PvPx2PIe/project/intelligence/agents/long.md:3 description is 1100 chars — over the 1024-char limit; the skill/agent will be REJECTED at load time".
4 sync --compact > replay.log 2>&1; repeat warning grep. PR script pass Exit 0; the exact same WARNING: line appeared on the next unchanged compact run.
5 Create demo/real.md with effort: ultra, symlink demo/SKILL.md to it, then sync. PR script pass Exit 0; "IS_STATUS=ok IS_DETAIL=synced=4". Claude, Codex and Copilot each printed "skill: demo".
6 Require Claude SKILL.md to be a regular file with effort: max. PR script pass Both file predicates returned 0; grep -Fx printed "effort: max".
7 Require Copilot SKILL.md to be regular without effort:; require no links under .claude .github .agents. PR script pass Both file predicates returned 0; effort grep returned no match; find .claude .github .agents -type l returned empty, and emptiness assertion passed.
8 Create empty outside directory, link intelligence/skills/escape to it, sync --compact, require specified warning and no escape output. PR script pass Exit 0; exact output "WARNING: skill 'escape' is left out of every output: intelligence/skills/escape is a symlink that resolves outside intelligence/skills". All three absence assertions passed for .claude/skills, .github/skills, .agents/skills.
9 bash "$CLI" status --check. PR script pass Exit 0; engine 0.20.0, matching schema and all four contracts checked, ending "all good.".

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.

Failures

None.

@dzykovic dzykovic added the ai:verified Declared behavior verified on the reported head label Oct 7, 2026
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Code review - CLEAN

head: f33d1a7 - base: main (f9a44ec) - files 26, +907/-133

Critical

None.

Warning

None.

Suggestion

None.

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.

@dzykovic dzykovic added the ai:reviewed Diff reviewed on the reported head label Oct 7, 2026
@dzykovic dzykovic added ai:completed Agent work completed; required factors are fresh and removed ai:processing An agent is working on this pull request labels Oct 7, 2026
@dzykovic
dzykovic merged commit 367de7a into main Oct 7, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:completed Agent work completed; required factors are fresh ai:reviewed Diff reviewed on the reported head ai:verified Declared behavior verified on the reported head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A skill directory reached through a symlink is left out of every output without a diagnostic

2 participants