Skip to content

Let sources name a package's directories by full name or alias - #48

Merged
dzykovic merged 12 commits into
mainfrom
feature/package-source-token
Oct 7, 2026
Merged

dzykovic merged 12 commits into
mainfrom
feature/package-source-token

Conversation

@dzykovic

@dzykovic dzykovic commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

sources: could name an installed package's content only by its store path, .intelligence/packages/@ainova-systems/sync/rules. A person can now name it by the package instead, in two more spellings, while the store path stays valid and stays what the CLI writes:

  • @scope/name/<dir> — folder <dir> of the package packages: declares as @scope/name. It is a reference only when its first two segments equal a declared package name; otherwise it is an ordinary path, exactly as before.
  • <alias>:<dir> — the same through a per-package alias, stored as alias: "<alias>" in that package's packages: entry and written only by package add --alias and the new package alias <@scope/name> <alias> | --remove.

The shape of the work:

  • Engine: one parser (IS_PKG_REF_AWK in engine/lib/common.sh) reads the names and aliases in packages: and expands a reference to its store path inside the awk pass read_yaml_list / load_yaml_lists already make — no extra process, and every reader sees the store path, so output is byte-identical. A reference that names nothing (unknown or shared alias, <dir> that is empty or leaves the package) is left out of the list and named in a sync WARNING: line.
  • CLI: nothing migrates and nothing rewrites. Wiring (init, add, update, restore, alignment, engine-content install) still writes store paths but leaves a directory a section already names in any spelling where it stands; package remove removes the package's entries in every spelling and its alias; update drops only the sections a new version lost and keeps the rest in place (it used to move the package first). Alias validation and uniqueness are checked before anything is fetched or written; removing or replacing an alias sources: uses is refused, naming the entries.
  • Checks and editors: status --check reports unresolved references, malformed or shared aliases, and one package directory listed under two spellings, and now judges an @... path like any project directory. source add|remove refuse references naming intelligence package and accept any spelling as an anchor. package list shows aliases.
  • Trailing comments: the shared list reader read - "x" # c as x # c in sources:, ignore: and submodules: (on main too), so a commented source was skipped in silence and status --check refused the #. One function, yaml_item in engine/lib/common.sh, now reads a list entry for the engine and for the CLI's sources: editors alike: a comment starts at a # after whitespace outside quotes, a # inside quotes stays part of the value, and whitespace outside the quotes is dropped. The editors used to cut #… after removing quotes, so they and the engine disagreed on "a #b"; they now agree (decision 0009, point 7).
  • Release: lines under the pending ## [0.19.0] ### Added and ### Fixed; no ### Breaking item from this PR. Decision 0019 records the design and what was rejected (short package: names, scope-only form, a prefix, implicit wiring, migration, 1.0.0). Decision 0005 is unchanged from main; intelligence/rules/cli.md changes only its "Who owns what" sentence, which now states that the engine reads paths and package references from sources: and the names and aliases of packages:.

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 groups: init, sync, update, upgrade, package, source, adapter, status, registry — alias is a package subcommand
  • Keeps preview/apply behavior explicit for planned writes
  • Covers automatic project alignment and CI refusal when project schema/content can change — alignment does not touch sources:
  • Keeps fresh-clone package restoration behind intelligence sync

Verification

  • bash cli/tests/verify.sh reports verify ok and skipped nothing this change needed — scope all (lint engine, lint cli, all 18 suites), run in WSL on the final tree
  • intelligence status --check succeeds in the relevant fixture/project
  • A second intelligence sync produces no unexpected diff
  • Adapter changes were checked against the real tool format and preserve hand-authored sibling files — n/a: no adapter changed; see the A/B in Notes
  • User-facing docs and examples use the current public command groups

Run each step in a POSIX shell (Git Bash, Linux or macOS) with CI unset. <CLI> = bash /d/Repositories/ainova/intelligence-package-source-token/cli/intelligence (any checkout of this branch: bash <checkout>/cli/intelligence).

  1. Store paths unchanged — from an empty git init directory: <CLI> init --targets claude exits 0; each sources: section begins with .intelligence/packages/@ainova-systems/sync/<section> followed by intelligence/<section>, exactly as main writes it; .claude/skills/intelligence-sync/SKILL.md exists; <CLI> status --check exits 0.
  2. Full-name form — from AC1's project: save find .claude -type f | sort | xargs sha256sum outside the project; run sed -i 's#"\.intelligence/packages/@ainova-systems/sync/#"@ainova-systems/sync/#' intelligence.yaml; <CLI> sync exits 0; the saved hashes still match (sha256sum -c); the sources: block is exactly as edited (no store path written back); <CLI> status --check exits 0.
  3. Alias form — from AC1's project with its .claude hashes saved: <CLI> package alias @ainova-systems/sync sync exits 0, the "@ainova-systems/sync" entry under packages: gains alias: "sync", and sources: is untouched; then sed -i 's#"\.intelligence/packages/@ainova-systems/sync/#"sync:#' intelligence.yaml; <CLI> sync exits 0; the saved hashes still match; sed -n '/^sources:/,/^[a-z]/p' intelligence.yaml | grep -c @ainova-systems prints 0; <CLI> status --check exits 0.
  4. Nothing writes the store path back — from AC3's project: save the sources: block; rm -rf .intelligence/packages, then <CLI> sync exits 0 and .claude/skills/intelligence-sync/SKILL.md exists; then set schema_version: "0.18.1" and run <CLI> sync: exits 0 and schema_version equals engine/VERSION. After both runs the sources: block is byte-identical to the saved one.
  5. Alias flag and uniqueness — from AC3's project plus a local git repository holding a committed rules/acme.md: <CLI> package add "git+file://<repo>" --name @acme/tools --alias sync exits non-zero naming @ainova-systems/sync as the holder of sync, leaving intelligence.yaml and intelligence.lock unchanged; --alias 'a/b' is refused the same way; <CLI> package add "git+file://<repo>" --name @acme/tools --alias acme exits 0, the "@acme/tools" entry has alias: "acme", and .claude/rules/acme.md exists.
  6. Unknown alias — from AC3's project with - "nope:rules" hand-added to sources.rules: <CLI> sync exits 0 and prints a WARNING: line naming nope:rules; <CLI> status --check exits non-zero naming nope:rules. With that line replaced by - "@acme/missing/rules" (no such declared package), <CLI> status --check treats it as an ordinary project path: exit 0, reported as not created yet.
  7. Removing a package removes its references — from AC5's project with its .intelligence/packages/@acme/tools/rules entry replaced by acme:rules: <CLI> package remove @acme/tools exits 0; no acme: entry, no @acme/tools entry and no alias: "acme" remain; .claude/rules/acme.md is gone; <CLI> status --check exits 0.
  8. source command — from AC3's project with docs/rules created: <CLI> source add rules docs/rules --after sync:rules yields the order sync:rules, docs/rules, intelligence/rules; <CLI> source add rules @ainova-systems/sync/rules and <CLI> source remove rules sync:rules are each refused, naming intelligence package.
  9. Docs and release — ## [0.18.1] in CHANGELOG.md is byte-identical to origin/main; ## [0.19.0] holds Let sources name a package's directories by full name or alias #48's lines under ### Added (fixes under ### Fixed) and no ### Breaking item from Let sources name a package's directories by full name or alias #48; docs/cli.md describes the three spellings, the alias command, and that a CLI ≤ 0.18 reads a hand-written reference as a missing path; decisions/0019-* records the design; decisions/0005-* is byte-identical to origin/main; intelligence/rules/cli.md differs from origin/main only in the "Who owns what" sentence stating what the engine reads.
  10. Trailing comment — from AC1's project: change the sources.rules line - "intelligence/rules" to - "intelligence/rules" # project rules and create intelligence/rules/marker.md holding # Marker and COMMENT_MARKER; <CLI> sync exits 0 and .claude/rules/marker.md exists; <CLI> status --check exits 0; <CLI> source list shows the entry as intelligence/rules.

Automated coverage: steps 1–8 and 10 run in order, one heading per criterion, in cli/tests/e2e-sources.sh ("… (ACn)"), with portable stand-ins for sha256sum and sed -i so the suite runs on macOS. The parser's states, expansion, cache and alias reading, and the trailing-comment regression for sources:, ignore: and submodules: (it fails on origin/main), are in cli/tests/unit-engine.sh; the editors and alias helpers (sources_remove_package, package_source_listed, sources_find_entry, alias validation, uniqueness and in-use refusal, and the editors reading a commented or #-holding entry exactly as the engine does) in cli/tests/unit-manifest.sh; update keeping entries in place, dropping a lost section spelled by alias, and a restore leaving a full-name entry as written in cli/tests/e2e-packages.sh. Step 9 is prose and release state, checked with git diff origin/main -- decisions/0005-* (empty), git diff origin/main -- intelligence/rules/cli.md (the one sentence) and a diff of the ## [0.18.1] section.

Versioned artifacts

  • The PR records its change under one concrete CHANGELOG.md version; there is no [Unreleased] section or duplicated release date — 0.19.0, pending
  • The current engine/VERSION release state was checked: append while pending, or select the next SemVer after publication — v0.19.0 has no published GitHub Release, so the lines join the existing section
  • If a new version was selected, every example's schema_version and exact @ainova-systems/sync pin changed with it — n/a: no new version selected
  • No stable release is implied or published without owner approval

Notes for reviewers

  • Byte identity, A/B: one project with @ainova-systems/sync and a local @acme/tools (rules, agents, skills), eight targets (agents,claude,cursor,codex,copilot,opencode,antigravity,pi), 98 output files. main's CLI on store paths, and this branch on store paths, with full names, with aliases, and with a trailing # comment on every sources: entry (sync --force each) produce identical hashes for every output file.
  • A CLI ≤ 0.18 reads a hand-written reference as a path that does not exist and skips it, and its restore or alignment wires the store path beside it; a current status --check then reports the directory as listed twice. References are opt-in, so nothing changes for a manifest nobody edits — the docs, the CHANGELOG line and decision 0019 say so.
  • package remove now also removes a hand-written store path to a non-standard directory of the package, which it used to leave dangling. status --check used to skip every @... entry (a legacy remote-token remnant); it now judges an unmatched one as an ordinary path, which criterion 6 requires.
  • Not changed here: the same trailing-comment reading exists in the Copilot and Pi adapters' parsers of a rule's frontmatter paths: list (engine/adapters/copilot.sh, pi.sh) and in legacy conversion's line rewriter, which splits a comment even inside quotes. None of them is served by read_yaml_list, so they are outside this fix. The CLI editors also accept an item line at any indentation (^[ \t]*-) where the engine reads ^ - only; the two differ only on YAML that is invalid anyway.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 11:59
@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.

Copilot review overview

🟡 Changes recommended

Undeclared-token behavior is inconsistent, and the minor release contradicts the repository’s compatibility contract.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Introduces package:<name>/<dir> source tokens across parsing, lifecycle operations, migration, documentation, and examples.

Changes:

  • Expands and validates package source tokens.
  • Canonicalizes tokens during package operations and migration.
  • Promotes artifacts to 0.19.0 with comprehensive tests and documentation.
File Description
packages/​sync/​skills/​intelligence-update-context/​SKILL.md Recognizes package-token sources.
packages/​sync/​skills/​intelligence-review-context/​SKILL.md Clarifies upstream package findings.
packages/​sync/​references/​conventions.md Documents token syntax and migration.
packages/​sync/​references/​adapters.md Documents parser-expanded adapter inputs.
examples/​with-remote-skills/​intelligence.yaml Migrates remote-package example.
examples/​platform-with-submodules/​intelligence.yaml Migrates platform example.
examples/​platform-with-submodules/​intelligence.lock Updates lock to 0.19.0.
examples/​go-api/​intelligence.yaml Migrates Go example.
examples/​go-api/​intelligence.lock Updates lock to 0.19.0.
examples/​go-api-with-pi-and-codex/​intelligence.yaml Migrates Pi/Codex example.
examples/​go-api-with-pi-and-codex/​intelligence.lock Updates lock to 0.19.0.
examples/​go-api-with-opencode/​intelligence.yaml Migrates OpenCode example.
examples/​go-api-with-opencode/​intelligence.lock Updates lock to 0.19.0.
examples/​go-api-with-antigravity/​intelligence.yaml Migrates Antigravity example.
examples/​go-api-with-antigravity/​intelligence.lock Updates lock to 0.19.0.
examples/​dotnet-api-with-react-frontend/​intelligence.yaml Migrates .NET/React example.
examples/​dotnet-api-with-react-frontend/​intelligence.lock Updates lock to 0.19.0.
examples/​cli-project/​intelligence.yaml Migrates multi-package example.
engine/​VERSION Promotes engine to 0.19.0.
engine/​sync.sh Warns about unresolved tokens.
engine/​lib/​common.sh Implements token parsing and expansion.
engine/​adapters/​_template.sh Documents expanded source lists.
docs/​cli.md Documents public token behavior.
decisions/​0015-name-package-sources-by-package.md Records the architecture decision.
cli/​tests/​unit-manifest.sh Tests canonical spelling and wiring.
cli/​tests/​unit-engine.sh Tests parsing and classification.
cli/​tests/​e2e-sources.sh Tests source commands and diagnostics.
cli/​tests/​e2e-packages.sh Tests collisions and stable ordering.
cli/​tests/​e2e-lifecycle.sh Tests migration and output identity.
cli/​lib/​registry.sh Wires packages using tokens.
cli/​lib/​manifest.sh Implements canonical token rewriting.
cli/​lib/​cli-common.sh Integrates expanded store checks.
cli/​internal/​package-update.sh Preserves source ordering during updates.
cli/​internal/​package-remove.sh Unwires packages by name.
cli/​internal/​package-add.sh Handles token collisions during add.
cli/​internal/​convert-legacy.sh Converts legacy paths to tokens.
cli/​internal/​check.sh Diagnoses invalid package tokens.
cli/​internal/​align-project.sh Adds lifecycle token migration.
cli/​commands/​source.sh Handles tokens in source operations.
cli/​commands/​init.sh Updates generated manifest guidance.
CHANGELOG.md Records 0.19.0 behavior and breakage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread engine/lib/common.sh Outdated
Comment thread decisions/0015-name-package-sources-by-package.md Outdated
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Verification - PASS

head: 5bb148a - steps: PR §Verification — checklist (5 items) + numbered steps AC1-AC7; qa_checks: none matched (no docs/qa-checks.md in this repo) - env: local (bash /d/Repositories/ainova/intelligence-package-source-token/cli/intelligence, built from head; no PR preview/deployment exists)

# Step Source Verdict Observed
1 bash cli/tests/verify.sh reports verify ok, skipping nothing this change needed — scope all, run in WSL on the final tree PR (checklist) pass Ran scope all on this worktree at head: lint: engine ok, lint: cli ok, all 18 suites ALL OK/OK/checks passed (unit-semver, unit-manifest, unit-engine, unit-gitignore, unit-release, unit-fetch, unit-upgrade, unit-verify, e2e-sources, e2e-packages, e2e-lifecycle, e2e-negative, e2e-lock-validation, e2e-compat, e2e-sync-cache, e2e-concurrency, e2e-sync-lock, e2e-store-sha), final line === verify ok ===, exit 0. Only skip seen: a Windows-junction case inside e2e-sync-cache skipped because the WSL host reports non-Windows — a host-conditional skip unrelated to this change, not one it needed.
2 intelligence status --check succeeds in the relevant fixture/project PR (checklist) pass Exited 0 on every fixture built for AC1-AC6 below (new project, migrated project, collision project, post-removal project), each printing all good.
3 A second intelligence sync produces no unexpected diff PR (checklist) pass On the AC2 fixture, hashed every tracked file, ran sync again: printed Unchanged: generated files are up to date. / === Unchanged: no files needed updating ===, exit 0; post-hash diff empty.
4 Adapter changes were checked against the real tool format and preserve hand-authored sibling files PR (checklist, marked n/a: "no adapter changed", A/B in Notes) pass Diffed main...head for adapter-related paths: only engine/adapters/_template.sh and packages/sync/references/adapters.md changed, both comment/reference-doc updates describing token expansion (no per-IDE adapter logic touched). Confirms the PR's own n/a claim; nothing to check against a real tool format.
5 User-facing docs and examples use the current public command groups PR (checklist) pass docs/cli.md has a "Package sources" section describing package:<name>/<dir> with store-path comments (lines ~390-405); remaining legacy mentions refer to the pre-existing, unrelated Intelligence Sync conversion feature, not stale command groups.
6 AC1 New project — init --targets claude exits 0; each sources: section begins package:sync/<section> then intelligence/<section>; @ainova-systems count in sources is 0; .claude/skills/intelligence-sync/SKILL.md exists; status --check exits 0 PR pass From an empty git init dir: init --targets claude exit 0; sources: = package:sync/rules+intelligence/rules (and agents/skills likewise); grep count 0; SKILL.md present; status --check → all good., exit 0.
7 AC2 0.18 project migrates — hashes before, edit manifest to 0.18 shape, sync exits 0 and prints a migration line, entries respelled back to tokens in the same positions, schema_version == engine/VERSION, .claude/ hashes unchanged, status --check exits 0 PR pass Edited manifest to store paths + schema_version: "0.18.0"; sync printed migrating: package store paths in sources -> package:<name>/<dir> and schema_version -> 0.19.0, exit 0; sources back to package:sync/<section> in the same order; schema_version 0.19.0 == engine/VERSION; .claude/ file hashes identical before/after; status --check → all good., exit 0.
8 AC3 Short-name collision — package add git+file://<repo> --name @acme/sync exits 0; sources.rules holds both full tokens, no bare package:sync/rules; .claude/rules/acme.md exists; status --check exits 0 PR pass Built a local git repo with committed rules/acme.md; package add "git+file://…" --name @acme/sync exit 0; sources.rules = package:@acme/sync/rules, package:@ainova-systems/sync/rules, intelligence/rules (no bare package:sync/rules); .claude/rules/acme.md exists; status --check → all good., exit 0.
9 AC4 Collision removed — package remove @acme/sync leaves no @acme/sync entry, survivors read package:sync/<section> again, .claude/rules/acme.md gone PR pass package remove @acme/sync exit 0; sources.rules back to package:sync/rules, intelligence/rules only; .claude/rules/acme.md removed.
10 AC5 Unknown token — hand-add package:nope/rules; status --check exits non-zero and names it as matching no declared package PR pass Added the line; status --check printed ✗ sources.rules 'package:nope/rules' matches no declared package — 'intelligence package add' installs one, or correct the entry, 1 problem(s)., exit 1.
11 AC6 source command — source add rules package:sync/rules refused naming package add; source remove rules package:sync/rules refused naming package remove; source add rules docs/rules --after package:sync/rules yields order package:sync/rules, docs/rules, intelligence/rules PR pass source add → ERROR: 'package:sync/rules' names installed package content — install it with 'intelligence package add'…, exit 1. source remove → ERROR: 'package:sync/rules' is installed package content — remove the package instead: intelligence package remove <@scope/name>, exit 1. source add rules docs/rules --after package:sync/rules → exit 0, resulting sources.rules order exactly package:sync/rules, docs/rules, intelligence/rules.
12 AC7 Docs — CHANGELOG.md has ## [0.19.0] with a ### Breaking item on CLI ≥0.19.0; docs/cli.md describes the package: form PR pass CHANGELOG.md ## [0.19.0] section has a ### Breaking block: "Every teammate and CI job that syncs the project runs CLI 0.19.0 or later… a CLI 0.18 or earlier reads a package: source as a path… renders a migrated project without package content…". docs/cli.md has a dedicated "Package sources" section with the token grammar and examples.

Negative probes (beyond the declared steps, per passing step 10/11's boundary)

  • Ambiguous token: hand-edited a project with both @acme/sync and @ainova-systems/sync declared to use the bare package:sync/rules token. status --check correctly reported ✗ … is ambiguous: @ainova-systems/sync, @acme/sync are all named 'sync' — name one in full…, exit 1 (distinct wording from the "unknown" case in AC5, matching the PR's own claim that status --check distinguishes unknown/ambiguous/malformed).
  • Malformed token: hand-added a bare "package:" entry. status --check reported ✗ 'package:' is not a package source — expected package:<name>/<dir>, exit 1.
  • Unplaceable anchor: source add rules more-docs --after package:nope/rules (an anchor naming an undeclared package). Refused: ERROR: sources.rules does not list 'package:nope/rules'., exit 1, no write.

All three negative probes behaved correctly; no defects found.

No qa_checks entries matched this PR's file list (no docs/qa-checks.md exists in this repository, so no standing checks apply beyond the declared steps above).

@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: 5bb148a - base: main - files 41, +1146/-200

Critical

(none)

Warning

(none)

Suggestion

  • cli/internal/check.sh:197-199 / engine/lib/common.sh (pkt_parse) - the undeclared state (a full package:@scope/name/dir token naming a package packages: does not declare) is exercised only by the read_source_entries unit test in cli/tests/unit-engine.sh (package:@acme/undeclared/rules|undeclared|...); no e2e test drives status --check or sync through this exact state the way AC5 does for unknown. Not a correctness gap - check.sh's case arm for unknown|undeclared is straightforward and symmetric - just a thinner acceptance-level trace than the sibling states get.

Claims axis

  • Every behavior the body claims has delivering code: engine expansion in read_yaml_list/load_yaml_lists (engine/lib/common.sh), package_sources_respell as the one CLI writer (cli/lib/manifest.sh, wired from package-add.sh, package-remove.sh, package-update.sh, registry.sh, align-project.sh, convert-legacy.sh), migration 3's byte-identical rewrite, status --check/sync reporting for unknown/ambiguous/malformed tokens, and the update ordering fix - all present and covered by the automated tests the body names (verified below).
  • Deployment notes: CHANGELOG.md's ### Breaking section names the exact migration risk (an older CLI silently drops package content) and the required manifest state after alignment - matches decision 0015 point 7's deliberate exception to decision 0005.
  • No undeclared scope: the diff stays within what the body describes (engine parser, CLI respelling, checks/editors, docs, examples, the one decision record).
  • This repository's own intelligence.yaml/intelligence.lock are untouched and still store-path/0.18.1, matching the body's note that they migrate only when the global CLI is upgraded.

Verification performed

  • bash cli/tests/unit-manifest.sh, bash cli/tests/unit-engine.sh, bash cli/tests/e2e-sources.sh, bash cli/tests/e2e-packages.sh, bash cli/tests/e2e-lifecycle.sh (native and WSL): all pass.
  • bash cli/tests/verify.sh lint: ok.
  • bash cli/tests/verify.sh tests (full 18-suite scope, WSL): verify ok, including e2e-lifecycle and e2e-negative (the two suites this host otherwise can't verify per CLAUDE.md).
  • Spot-checked pkt_name_ok (engine awk) against assert_valid_pkg_name (CLI bash) - same charset/shape rule reimplemented across the bash/awk boundary, and the awk comment names assert_valid_pkg_name as its source of truth, satisfying the single-source-of-truth rule's inline-restatement carve-out.
  • Confirmed decision 0015 is accepted, cites the real constraints (order-is-override from decision 0009, the breaking exception to decision 0005), and its ### Breaking CHANGELOG item matches.
  • No secrets in the diff; single commit, title in the required past-tense/capitalized form, matches the PR title.

Summary

  • files: 41, +1146/-200; critical: 0; warning: 0; suggestion: 1
  • verdict: CLEAN

@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: c1bbbc8 - steps: PR §Verification — checklist (5 items) + numbered steps AC1-AC7; qa_checks: none matched (no docs/qa-checks.md in this repo) - env: local (bash /d/Repositories/ainova/intelligence-package-source-token/cli/intelligence, built from head; no PR preview/deployment exists)

Re-verified against the new head (prior reports on this PR name 5bb148a1b69b594840e828c3c6231d00cd622f78, a different commit). Since that head, the branch added 429c2ec (full-form package:@scope/name/dir tokens naming an undeclared package no longer expand to the store path — they skip like an unknown token) and merged main (c1bbbc8b, unrelated Codex invocation-policy work, no conflict with this change's files).

# Step Source Verdict Observed
1 bash cli/tests/verify.sh reports verify ok, skipping nothing this change needed — scope all, run in WSL on the final tree PR (checklist) pass Ran scope all on this worktree at head: lint: engine ok, lint: cli ok, all 18 suites ALL OK/checks passed, final === verify ok ===, exit 0. No FAIL lines; only skip was the host-conditional Windows-junction case inside e2e-sync-cache (non-Windows WSL host), unrelated to this change.
2 intelligence status --check succeeds in the relevant fixture/project PR (checklist) pass Exited 0 on every fixture built for AC1-AC4 below, each printing all good.
3 A second intelligence sync produces no unexpected diff PR (checklist) pass Fresh init --targets claude, then sync run twice more: the first explicit sync call after init re-rendered (no prior cache record exists yet — pre-existing sync-cache behavior, untouched by this PR's files); the next sync printed Unchanged: generated files are up to date. / === Unchanged: no files needed updating ===, exit 0, and a full file hash pass before/after that call was byte-identical.
4 Adapter changes were checked against the real tool format and preserve hand-authored sibling files PR (checklist, marked n/a: "no adapter changed") pass git diff main...HEAD for engine/adapters/* and packages/sync/references/adapters.md touches only a doc-comment line in _template.sh (describes that read_yaml_list now hands back expanded tokens) and a reference-doc line; no adapter logic changed. Confirms the n/a claim.
5 User-facing docs and examples use the current public command groups PR (checklist) pass docs/cli.md:399 describes package:<name>/<dir> in its current (post-429c2ec) wording; CHANGELOG.md ## [0.19.0] matches.
6 AC1 New project — init --targets claude exits 0; each sources: section begins package:sync/<section> then intelligence/<section>; @ainova-systems count in sources is 0; .claude/skills/intelligence-sync/SKILL.md exists; status --check exits 0 PR pass From an empty git init dir: init --targets claude exit 0; sources: = package:sync/rules+intelligence/rules (and agents/skills likewise); grep count 0; SKILL.md present; status --check → all good., exit 0.
7 AC2 0.18 project migrates — hashes before, edit manifest to 0.18 shape, sync exits 0 and prints a migration line, entries respelled back to tokens in the same positions, schema_version == engine/VERSION, .claude/ hashes unchanged, status --check exits 0 PR pass Edited manifest to store paths + schema_version: "0.18.0"; sync printed migrating: package store paths in sources -> package:<name>/<dir> and schema_version -> 0.19.0, exit 0; sources back to package:sync/<section> in the same order; schema_version 0.19.0 == engine/VERSION; .claude/ file hashes identical before/after; status --check → all good., exit 0.
8 AC3 Short-name collision — package add git+file://<repo> --name @acme/sync exits 0; sources.rules holds package:@acme/sync/rules and package:@ainova-systems/sync/rules and no bare package:sync/rules; .claude/rules/acme.md exists; status --check exits 0 PR pass Local git repo with committed rules/acme.md; package add "git+file://<repo>" --name @acme/sync exit 0; sources.rules = package:@acme/sync/rules, package:@ainova-systems/sync/rules, intelligence/rules; .claude/rules/acme.md present; status --check → all good., exit 0.
9 AC4 Collision removed — package remove @acme/sync leaves no @acme/sync entry, survivors read package:sync/<section> again, .claude/rules/acme.md is gone PR pass package remove @acme/sync exit 0; sources.rules back to package:sync/rules, intelligence/rules; .claude/rules/acme.md gone.
10 AC5 Unknown token — hand-add "package:nope/rules" to sources.rules; status --check exits non-zero and names it as matching no declared package PR pass Added the line; status --check printed ✗ sources.rules 'package:nope/rules' matches no declared package — 'intelligence package add' installs one, or correct the entry, 1 problem(s)., exit 1.
11 AC6 source command — source add rules package:sync/rules refused naming package add; source remove rules package:sync/rules refused naming package remove; source add rules docs/rules --after package:sync/rules yields order package:sync/rules, docs/rules, intelligence/rules PR pass source add → exit 1, ERROR: 'package:sync/rules' names installed package content — install it with 'intelligence package add'...; source remove → exit 1, ERROR: 'package:sync/rules' is installed package content — remove the package instead: intelligence package remove <@scope/name>; source add rules docs/rules --after package:sync/rules → exit 0, resulting order package:sync/rules, docs/rules, intelligence/rules.
12 AC7 Docs — CHANGELOG.md has ## [0.19.0] with a ### Breaking item on CLI ≥ 0.19.0; docs/cli.md describes the package: form PR pass CHANGELOG.md ## [0.19.0] → ### Breaking states "Every teammate and CI job that syncs the project runs CLI 0.19.0 or later... a CLI 0.18 or earlier reads a package: source as a path, so it renders a migrated project without package content..."; docs/cli.md:399-405 describes the token form and the same CLI-version requirement.

Negative probes (beyond the declared steps, per the boundary each step names)

  • Full-form token naming an undeclared package (package:@acme/gone/rules), store directory present: status --check reports matches no declared package (same as the short-form case in step 10); sync prints the WARNING: ... resolves to no single declared package — skipped and does not render the store content. This is the exact behavior 429c2ec added since the prior-head report — confirmed by hand, not just by reading the automated test (cli/tests/e2e-sources.sh) that also covers it.
  • source add rules docs2/rules --after package:nonexistent/rules (anchor not present in sources.rules): refused cleanly — ERROR: sources.rules does not list 'package:nonexistent/rules'. — manifest left unchanged.

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 - BLOCK

head: c1bbbc8 - base: main - files 41, +1161/-200

Critical

  • decisions/0015-name-package-sources-by-package.md:60-63 contradicts decisions/0005-gate-schema-compatibility-on-the-major-version.md:28-29 without amending or superseding it. Decision 0005 point 4 is unconditional ("additive within a major... is released as a major bump") and its Status: is still accepted — unchanged by this PR. The same policy's generated-rule mirrors, intelligence/rules/cli.md:111 and AGENTS.md:98 ("Breaking changes follow expand-contract: add the new path, migrate consumers, then remove the old"), are also left untouched, so the repository now states the policy two contradictory ways: a reader at either rule file has no way to discover the carve-out decision 0015 grants itself.
    • An independent automated reviewer raised exactly this on review thread PRRT_kwDOUE7J2c6p5Kqb (#discussion_r4206640630), naming the same three sources. The author's reply (#discussion_r4206965772), "Escalated to the owner... is a release-policy decision, listed in this PR's outcome comment," does not resolve it: no comment containing an outcome section exists on this PR as of this head (only the ## Verification - PASS and ## Code review - CLEAN stage comments already posted), decision 0005 was not amended or marked superseded, and the thread is marked resolved with no decision actually recorded anywhere in-repo.
    • The practical effect, stated in the PR's own additions (docs/cli.md's new "One shape breaks this within the 0.x major..." sentence and CHANGELOG.md's ### Breaking checklist): a teammate still on 0.18 who syncs a migrated project renders it silently without its package content and still gets IS_STATUS=ok — the exact failure mode decision 0005 point 4 exists to turn into a loud major-version refusal instead. The only guard shipped is an unchecked changelog checklist item: a hand-read, hand-ticked gate, not a code gate (dev-verification-gates, dev-skill-first).
    • Expand-contract is not followed either (dev-rollback-safety.md): the engine gains the ability to read package: tokens and lifecycle alignment auto-migrates every existing manifest to that shape in the same release, 0.19.0 — no window exists in which consumers are upgraded before the new, incompatible shape ships. Decision 0015's own "Rejected" section (lines 85-97) weighs only "Releasing as 1.0.0" against "No automatic migration" (hand-edit); it never considers staging the reader in one release and the migration in a later one — the actual expand-contract shape the project's own rule requires, which would resolve the conflict without either rejected alternative.
    • Fix: amend decision 0005 (supersede point 4, or add an explicit, scoped carve-out) and both rule mirrors in the same change, or follow expand-contract — ship the reader this release, the migration a release later — or take the 1.0.0 bump decision 0015 itself names as the rule-compliant alternative.

Warning

(none)

Suggestion

(none — the prior review's one suggestion on this PR, the thin acceptance-level trace for the undeclared token state, was addressed in 429c2ec with a dedicated e2e-sources.sh case and is no longer outstanding)

Claims axis

  • Every other behavior the body claims has delivering code, re-verified directly: engine expansion (engine/lib/common.sh pkt_parse/pkt_expand/pkt_store/pkt_spell, exercised by unit-engine.sh), package_sources_respell as the one CLI writer (cli/lib/manifest.sh, wired from package-add.sh, package-remove.sh, package-update.sh, registry.sh, align-project.sh, convert-legacy.sh, cli-common.sh), migration 3's byte-identical rewrite (e2e-lifecycle.sh's new "a 0.18 manifest migrates..." case), the update ordering fix (e2e-packages.sh's new "update keeps a package's sources where they stand" case), and status --check/source list/sync reporting across ok/undeclared/unknown/ambiguous/invalid tokens (e2e-sources.sh, check.sh, source.sh).
  • The body frames the decision-0005 exception as settled ("recorded in decision 0015 with the alternatives weighed") without disclosing that an automated reviewer already objected to it on a rule-conflict basis and that the objection was only escalated, not resolved — a reader of the body alone has no way to know this is still an open call.
  • Deployment notes: the ### Breaking checklist does name the migration risk and the required post-alignment state; it is the only place the schema-compatibility conflict surfaces at all, which is why the unresolved thread above matters.
  • No undeclared scope otherwise: the diff stays within what the body describes.

Verification performed

  • Re-read engine/lib/common.sh's token-state functions and every caller (cli/lib/manifest.sh, cli/commands/source.sh, cli/internal/check.sh, engine/sync.sh) and confirmed the ok/undeclared/unknown/ambiguous/invalid states are handled consistently with decision 0015 everywhere they're consumed.
  • Ran independently in the judged worktree at this head: bash cli/tests/unit-engine.sh, bash cli/tests/unit-manifest.sh, bash cli/tests/e2e-sources.sh, bash cli/tests/e2e-packages.sh — all ALL OK. bash cli/tests/verify.sh lint — verify ok.
  • Confirmed CI (gh pr checks 48) is green at this head: cli-e2e (ubuntu/macos), cli-shellcheck, repo-purity, npm-pack-smoke, guardrails, and the per-example smoke matrix.
  • Scanned the full base...head diff for credential-like strings — none found.
  • Confirmed the judged worktree stayed at c1bbbc8b9b6d8a51e4084fe8e0a8716a8a9e6cc7, unmoved, for the duration of this review.

Summary

  • files: 41, +1161/-200; critical: 1; warning: 0; suggestion: 0
  • verdict: BLOCK

@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Outcome - MANUAL

head: c1bbbc8 - rounds: 2 of 3 - CI green - ai:verified fresh - ai:reviewed withheld - review threads: 0 unresolved

  • Release level against the compatibility contract (Critical in the code review at this head; Copilot thread r4206640630). Decision 0015 point 7 ships package: sources as minor 0.19.0, but a same-major CLI 0.18 or earlier renders a migrated project without package content and still reports IS_STATUS=ok. That contradicts decision 0005 point 4 and intelligence/rules/cli.md ("a shape an older engine of the same major cannot read is a major change"), and the migration ships with the reader instead of following expand-contract (dev-rollback-safety). The owner picks one: (a) keep 0.19.0 and amend the policy in this PR by rewording the cli.md sentence and superseding 0005 point 4 with a scoped carve-out; (b) release as 1.0.0 so an older CLI refuses loudly (0015 calls 1.0.0 a separate decision); or (c) ship only the reader now and move migration 3, and the token spelling that init, package and examples write, to a later release. Then the change is made and review runs again for ai:reviewed.

@dzykovic dzykovic added ai:manual Needs an owner decision ai:processing An agent is working on this pull request and removed ai:processing An agent is working on this pull request ai:manual Needs an owner decision labels Oct 7, 2026
@dzykovic dzykovic removed the ai:verified Declared behavior verified on the reported head label Oct 7, 2026
@dzykovic dzykovic changed the title Named package sources by package instead of by store path Let sources name a package's directories by full name or alias Oct 7, 2026
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Verification - PASS

head: d76db0c - steps: PR §Verification — checklist (5 items, 1 marked n/a) + numbered steps 1-10; qa_checks: none matched (no docs/qa-checks.md in this repo) - env: local (bash /d/Repositories/ainova/intelligence-package-source-token/cli/intelligence, built from head; no PR preview/deployment exists)

# Step Source Verdict Observed
1 bash cli/tests/verify.sh reports verify ok, skipping nothing this change needed — scope all PR (checklist) pass Ran scope all on this worktree at head (WSL): lint: engine ok, lint: cli ok, all 18 suites OK, final === verify ok ===, exit 0. No FAIL lines. Only skip: the host-conditional Windows-junction case in e2e-sync-cache (non-Windows WSL host) — unrelated to this change.
2 intelligence status --check succeeds in the relevant fixture/project PR (checklist) pass all good. / exit 0 on every fixture built for steps 1-4, 7, 8, 10 below.
3 A second intelligence sync produces no unexpected diff PR (checklist) pass Confirmed at step 4 below: second sync after the schema_version edit printed Unchanged: generated files are up to date., exit 0; sources: block byte-identical before/after.
4 Adapter changes checked against the real tool format and preserve hand-authored sibling files PR (checklist, marked n/a: "no adapter changed") pass git diff origin/main on engine/adapters/_template.sh and packages/sync/references/adapters.md touches only doc-comment lines describing that read_yaml_list now hands back expanded tokens — no per-adapter logic changed. Confirms the n/a claim.
5 User-facing docs and examples use the current public command groups PR (checklist) pass docs/cli.md lists package alias <@scope/name> <alias> / --remove as a subcommand of package, no new top-level verb; matches the "Public CLI check" claim.
1 (AC) Store paths unchanged — empty git init dir: init --targets claude exits 0; each sources: section begins .intelligence/packages/@ainova-systems/sync/<section> then intelligence/<section>; .claude/skills/intelligence-sync/SKILL.md exists; status --check exits 0 PR pass All confirmed exactly as stated.
2 (AC) Full-name form — from AC1: save .claude hashes; rewrite store paths to @ainova-systems/sync/...; sync exits 0; hashes still match; sources: block exactly as edited; status --check exits 0 PR pass sha256sum -c all OK; sources: unchanged by sync; status --check → all good., exit 0.
3 (AC) Alias form — package alias @ainova-systems/sync sync exits 0, packages: entry gains alias: "sync", sources: untouched; rewrite to sync:...; sync exits 0; hashes match; @ainova-systems count in sources: is 0; status --check exits 0 PR pass All confirmed exactly as stated; grep count 0.
4 (AC) Nothing writes the store path back — from AC3: save sources:; rm -rf .intelligence/packages, sync exits 0, SKILL.md exists; set schema_version: "0.18.1", sync exits 0, schema_version == engine/VERSION (0.19.0); sources: byte-identical to saved after both runs PR pass All confirmed; sync printed project alignment: stamp 0.18.1, engine 0.19.0 / schema_version -> 0.19.0; sources: diff empty.
5 (AC) Alias flag and uniqueness — package add ... --alias sync refused naming @ainova-systems/sync as holder, manifest/lock unchanged; --alias 'a/b' refused the same way; --alias acme exits 0, @acme/tools entry has alias: "acme", .claude/rules/acme.md exists PR pass ERROR: alias 'sync' already names @ainova-systems/sync..., exit 1, manifest/lock byte-identical; --alias 'a/b' → ERROR: invalid alias 'a/b'..., exit 1; --alias acme → exit 0, entry present, file present.
6 (AC) Unknown alias — hand-add "nope:rules"; sync exits 0 + WARNING: naming nope:rules; status --check exits non-zero naming nope:rules; replaced by "@acme/missing/rules" (undeclared package) — status --check treats it as an ordinary path: exit 0, not created yet PR pass All confirmed exactly as stated.
7 (AC) Removing a package removes its references — from AC5 with .intelligence/packages/@acme/tools/rules respelled acme:rules: package remove @acme/tools exits 0; no acme:/@acme/tools/alias: "acme" remain; .claude/rules/acme.md gone; status --check exits 0 PR pass All confirmed exactly as stated.
8 (AC) source command — from AC3 with docs/rules created: source add rules docs/rules --after sync:rules yields sync:rules, docs/rules, intelligence/rules; source add rules @ainova-systems/sync/rules and source remove rules sync:rules each refused naming intelligence package PR pass Order exact match; both refusals: ERROR: ... 'intelligence package add' wires... and ERROR: ... remove the package instead: intelligence package remove ..., exit 1 each, sources: unchanged.
9 (AC) Docs and release — ## [0.18.1] byte-identical to origin/main; ## [0.19.0] holds #48's lines under ### Added/### Fixed, no ### Breaking; docs/cli.md describes the three spellings/alias command/CLI≤0.18 behavior; decisions/0019-* exists; decisions/0005-* byte-identical to origin/main; intelligence/rules/cli.md differs only in the "Who owns what" sentence PR pass ## [0.18.1] section diff empty; ## [0.19.0] diff adds exactly 3 ### Added bullets + 2 ### Fixed bullets, no Breaking string anywhere in the diff; docs/cli.md covers all three spellings, package alias, and the ≤0.18 behavior; decisions/0019-name-package-sources-by-package.md exists; decisions/0005-* diff empty; intelligence/rules/cli.md diff is exactly the one "Who owns what" sentence.
10 (AC) Trailing comment — from AC1: sources.rules entry gains # project rules; create intelligence/rules/marker.md; sync exits 0, .claude/rules/marker.md exists; status --check exits 0; source list shows the entry as intelligence/rules PR pass All confirmed; source list prints 2. intelligence/rules with no trailing comment shown.

Negative probes (beyond the declared steps, per the boundaries steps 1(AC)/6(AC)/8(AC) name)

  • @scope/name/<dir> with an empty <dir> ("@ainova-systems/sync/"): sync prints WARNING: ... names no directory inside its package ..., skipped; status --check reports the same, exit 1.
  • <alias>:<dir> escaping the package ("sync:../../../etc"): rejected identically — not followed outside the package, WARNING/status --check finding, no path traversal.
  • Two packages hand-edited to declare the same alias (sync), with sources: entries still using it: sync prints a WARNING: per affected entry naming both packages as ambiguous; status --check reports it at both the packages: declaration level and every sources: usage, 5 problems, exit 1.
  • package alias on an undeclared package: ERROR: package '@nonexistent/pkg' is not in the manifest ..., exit 1, no write.
  • package alias with an empty alias string: ERROR: invalid alias '' ..., exit 1.

All probes behaved correctly; no defects found.

No qa_checks entries matched this PR's file list (no docs/qa-checks.md exists in this repository).

@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: d76db0c - base: main - files 35, +1635/-153

Critical

(none)

Warning

(none)

Suggestion

  • cli/commands/source.sh assert_not_package_reference re-derives @scope/name from the expanded store path by slicing on / (pkg="${rest%%/*}/" ...) instead of having sources_classify_entry (cli/lib/manifest.sh) hand back PKR_NAME directly alongside state/dir. Correct today because assert_valid_pkg_name guarantees exactly one /, but it is a second, hand-rolled reading of a value pkr_parse already computed (engine/lib/common.sh).
  • decisions/0019-name-package-sources-by-package.md point 1 defines a reference as "first two segments equal a declared package name"; a bare @scope/name entry (no third segment) therefore satisfies that definition and is classified invalid rather than path (empty <dir> fails pkr_dir_ok). Harmless and consistent with "<dir> is one or more segments," but worth a one-line callout in the decision since it reads, on first pass, like such an entry should fall back to "ordinary path."

Claims axis

PR body and code agree. Spot-checked: IS_PKG_REF_AWK/yaml_item land exactly where the body says (engine/lib/common.sh); package alias, --alias, alias-uniqueness/in-use refusals exist (cli/internal/package-alias.sh, cli/lib/manifest.sh); wire_package_sources/sources_remove_package leave a spelling where it stands and remove every spelling respectively (cli/lib/registry.sh, cli/lib/manifest.sh); source add|remove refuse references and accept any spelling as an anchor (cli/commands/source.sh); status --check reports unresolved/ambiguous/invalid references and double-listed directories (cli/internal/check.sh); the trailing-comment fix is one yaml_item shared by the engine and the CLI editors. CHANGELOG lines sit under the existing pending ## [0.19.0] (v0.18.1 is already published, v0.19.0 is not — matches release.md); decisions/0005-* is byte-identical to origin/main; intelligence/rules/cli.md differs only in the stated "Who owns what" sentence; no example's schema_version moved, consistent with "no new version selected"; only engine/adapters/_template.sh's doc comment changed, consistent with the "no adapter changed" checkbox. No undeclared scope found.

Verification corroboration

Ran independently of the PR's own git-verify-pr report (already posted for this head): unit-engine, unit-manifest, e2e-sources and e2e-packages all pass natively on this host (CLI-E2E-SOURCES: ALL OK, CLI-E2E: ALL OK, etc.), and shellcheck --severity=warning over engine/ and cli/ is clean. Nothing observed contradicts the existing ## Verification - PASS comment at this same head.

Summary

  • files: 35, +1635/-153; critical: 0; warning: 0; suggestion: 2
  • verdict: CLEAN

@dzykovic dzykovic added ai:reviewed Diff reviewed on the reported head ai:completed Agent work completed; required factors are fresh ai:processing An agent is working on this pull request and removed ai:processing An agent is working on this pull request ai:completed Agent work completed; required factors are fresh labels 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

Code review - PASS-WITH-WARNINGS

head: c25f1e7 - base: main - files 35, +1635/-153

Critical

(none)

Warning

  • cli/commands/source.sh:46-56 (assert_not_package_reference) re-derives @scope/name from the expanded store path by slicing on / (pkg="${rest%%/*}/" ...) instead of having sources_classify_entry (cli/lib/manifest.sh) hand back PKR_NAME directly alongside state/dir. Correct today only because assert_valid_pkg_name guarantees exactly one / in a package name — it is a second, hand-rolled reading of a value pkr_parse already computed (engine/lib/common.sh). Carried over unaddressed from the review at head d76db0c.
  • decisions/0005-gate-schema-compatibility-on-the-major-version.md:28-29 (point 4, "a change an older engine of the same major cannot read is a breaking change and is released as a major bump") is still unconditional and carries no pointer to decision 0019. Decision 0019 explains at length, and at its owner's explicit direction (the ## Outcome - MANUAL comment at head c1bbbc8), why shipping opt-in @scope/name/<dir> / <alias>:<dir> syntax as 0.19.0 doesn't need a major bump — nothing auto-migrates or auto-respells an existing entry (confirmed: cli/internal/align-project.sh and cli/internal/convert-legacy.sh are both untouched by this diff), so only a person who deliberately hand-writes the new syntax is affected, and decisions/0019:116 itself cites decision 0005 as the reason auto-migration was rejected. But that reasoning is only discoverable by already knowing to open 0019 — a reader who starts at 0005, the single accepted source for this policy (dev-context-engineering: "one source per convention; everything else references it"), has no sign that an opt-in exception now exists, or that a project which adopts it still leaves a same-major 0.18 teammate silently rendering without the referenced package's content (honestly documented in CHANGELOG.md's 0.19.0 entry and docs/cli.md's "Package references" section, just not from 0005's side). The design changed substantively since the Critical finding this exact tension drew at head c1bbbc8 (no migration ships now, so the risk is opt-in and bounded rather than a default outcome of upgrading), and the owner was explicitly consulted — so this is now a documentation-completeness gap rather than a shipped contradiction: add a line to decision 0005 pointing at 0019's carve-out.

Suggestion

  • decisions/0019-name-package-sources-by-package.md:30-33 (point 1) defines a reference as "first two segments equal a declared package name"; a bare @scope/name entry (no <dir>) therefore reads, on a first pass, like it should fall back to "ordinary path" — but pkr_parse (engine/lib/common.sh) classifies it invalid (empty <dir> fails pkr_dir_ok). Harmless and consistent with "<dir> is one or more segments," but worth a one-line callout in the decision. Carried over unaddressed from the review at head d76db0c.

Claims axis

  • Every behavior the body claims has delivering code, verified directly: engine parsing/expansion (IS_PKG_REF_AWK, yaml_item in engine/lib/common.sh, wired through read_yaml_list / read_yaml_list_raw / read_source_entries / load_yaml_lists); package alias and package add --alias (cli/internal/package-alias.sh, cli/internal/package-add.sh); alias uniqueness and in-use refusal before any write (assert_alias_free / assert_alias_unused, cli/lib/manifest.sh); wire_package_sources / sources_remove_package leaving a chosen spelling in place and removing every spelling respectively (cli/lib/registry.sh, cli/lib/manifest.sh); source add|remove refusing references and accepting any spelling as a --before/--after anchor (cli/commands/source.sh); status --check reporting unresolved/ambiguous/invalid references and a directory listed twice (cli/internal/check.sh); the update ordering fix keeping a retained section in place (cli/internal/package-update.sh); the one shared yaml_item trailing-comment fix in the engine and the CLI's editors. All match the automated tests the body names.
  • No undeclared scope: the diff stays within what the body describes (engine parser, CLI respelling/wiring, checks/editors, docs, one decision record, CHANGELOG). engine/VERSION is untouched and no example's schema_version moved, matching the "Versioned artifacts" checklist ("no new version selected"); the example diffs touch only a comment sentence, never sources: itself.
  • Deployment notes: CHANGELOG.md's 0.19.0 ### Added/### Fixed entries name the opt-in syntax, the alias command and the trailing-comment fix; its ### Breaking checklist carries no item from this PR (read directly — its four items are all from the already-merged Codex-effort and Copilot-gitignore work). This resolves the specific release-notes objection a prior review on this PR raised: there is no migration in this diff for a release reader to miss.
  • The body's own checklist is accurate where it claims n/a: step 4 ("adapter changes...") is marked n/a, and engine/adapters/_template.sh's doc-comment-only change confirms no adapter logic changed.

Verification performed

  • Ran independently in the judged worktree at this head: bash cli/tests/unit-engine.sh, bash cli/tests/unit-manifest.sh, bash cli/tests/e2e-sources.sh — all ALL OK. bash cli/tests/verify.sh lint — ok (engine and cli).
  • Confirmed CI (gh pr checks 48) is green at this head across every job: cli-e2e (ubuntu-latest, macos-latest), cli-shellcheck, shellcheck, repo-purity, npm-pack-smoke (ubuntu/macos/windows), guardrails, and the per-example smoke matrix.
  • Scanned the full origin/main...HEAD diff for credential-like strings — none found.
  • Read cli/internal/align-project.sh and cli/internal/convert-legacy.sh directly: neither is touched by this diff, confirming the "nothing migrates" claim and the absence of the auto-migration path a prior review on this PR flagged as Critical.
  • Confirmed the judged worktree stayed at c25f1e731c0384503cee3ddb999fc88050e5401c, unmoved, for the duration of this review.

Summary

  • files: 35, +1635/-153; critical: 0; warning: 2; suggestion: 1
  • verdict: PASS-WITH-WARNINGS

@dzykovic dzykovic added the ai:reviewed Diff reviewed on the reported head label Oct 7, 2026
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Verification - PASS

head: c25f1e7 - steps: PR Verification section - checklist (5 items) + numbered steps AC1-AC10; qa_checks: none matched (no docs/qa-checks.md in this repo) - env: local (bash cli/intelligence on the worktree, built from head; no PR preview/deployment exists - confirmed 0 deployments via the GitHub API)

# Step Source Verdict Observed
1 bash cli/tests/verify.sh reports verify ok, skipping nothing this change needed - scope all, run in WSL on the final tree PR (checklist) pass Ran scope all on the worktree at head; harness auto-delegated to WSL (WSL: all on a copy of this tree). lint: engine ok, lint: cli ok; all 18 suites (unit-semver, unit-manifest, unit-engine, unit-gitignore, unit-release, unit-fetch, unit-upgrade, unit-verify, e2e-sources, e2e-packages, e2e-lifecycle, e2e-negative, e2e-lock-validation, e2e-compat, e2e-sync-cache, e2e-concurrency, e2e-sync-lock, e2e-store-sha) reported ALL OK / OK / checks passed; final line verify ok, exit 0. Only skip: a Windows-junction case inside e2e-sync-cache (non-Windows host) - a host-conditional skip unrelated to this change.
2 intelligence status --check succeeds in the relevant fixture/project PR (checklist) pass Exited 0 (all good.) on every fixture built for AC1-AC10 below, including after migration, alias rewrites, package add/remove and the trailing-comment case. The one deliberate negative inside AC6 correctly exits 1.
3 A second intelligence sync produces no unexpected diff PR (checklist) pass AC2: hashed .claude before rewriting sources: to full-name form, ran sync, hashes matched (sha256sum -c). AC4: sources: block byte-identical after rm -rf .intelligence/packages + sync and after a schema-downgrade + sync.
4 Adapter changes checked against the real tool format and preserve hand-authored siblings PR (checklist, marked n/a: no adapter changed, A/B in Notes) pass Diffed origin/main...head restricted to adapter paths: only engine/adapters/_template.sh (one comment line) and packages/sync/references/adapters.md changed - no per-IDE adapter logic. Confirms the n/a claim this PR makes.
5 User-facing docs and examples use the current public command groups PR (checklist) pass docs/cli.md documents all three spellings (store path, @scope/name/, :), package alias / package add --alias, and the <=0.18 behavior. Every legacy mention in docs/cli.md refers to the pre-existing, unrelated Intelligence Sync conversion feature, not a stale command group.
6 AC1 New project - init --targets claude exits 0; each sources: section begins .intelligence/packages/@ainova-systems/sync/
then intelligence/
; .claude/skills/intelligence-sync/SKILL.md exists; status --check exits 0
PR pass From an empty git init dir: exit 0; sources.rules = .intelligence/packages/@ainova-systems/sync/rules + intelligence/rules (agents/skills likewise); SKILL.md present; status --check -> all good., exit 0.
7 AC2 Full-name form - from the AC1 project, hash .claude, rewrite store paths to @ainova-systems/sync/, sync exits 0, hashes still match, sources: block exactly as edited, status --check exits 0 PR pass sync exit 0; sha256sum -c matched; sources: unchanged from the edit (no store path written back); status --check -> all good., exit 0.
8 AC3 Alias form - package alias @ainova-systems/sync sync exits 0, packages: entry gains alias: sync, sources: untouched; rewrite to sync:, sync exits 0, hashes still match, @ainova-systems count in sources: is 0, status --check exits 0 PR pass package alias exit 0, alias: sync added, sources: untouched beforehand; after rewrite sync exit 0, hashes matched, grep count 0, status --check -> all good., exit 0.
9 AC4 Nothing writes the store path back - rm -rf .intelligence/packages then sync exits 0 and SKILL.md exists; set schema_version to 0.18.1 then sync exits 0 and schema_version equals engine/VERSION; sources: byte-identical to the saved block after both PR pass First sync exit 0 (restored store from lock), SKILL.md present. Second sync after downgrade: exit 0, printed schema_version arrow 0.19.0, schema_version equals 0.19.0 equals engine/VERSION. sources: block identical to the pre-removal save (diff empty).
10 AC5 Alias flag and uniqueness - package add with --alias sync (held by @ainova-systems/sync) exits non-zero naming the holder, manifest/lock unchanged; --alias a/b refused the same way; package add with --alias acme exits 0, @acme/tools has alias: acme, .claude/rules/acme.md exists PR pass Collision: ERROR naming alias sync already names @ainova-systems/sync, exit 1, manifest and lock byte-identical to before. Malformed: ERROR invalid alias a/b, exit 1, manifest unchanged. Valid: exit 0, alias: acme present, .claude/rules/acme.md created.
11 AC6 Unknown alias - hand-add nope:rules; sync exits 0 with a WARNING naming it; status --check exits non-zero naming it; replaced by @acme/missing/rules (undeclared), status --check treats it as an ordinary path: exit 0, not created yet PR pass sync exit 0, WARNING line named sources.rules nope:rules and alias nope, which no package in packages: declares. status --check exit 1, same entry flagged as a problem. After substitution: status --check exit 0, entry reported as sources.rules @acme/missing/rules - not created yet (optional).
12 AC7 Removing a package removes its references - from the AC5 project with its @acme/tools entry respelled acme:rules; package remove @acme/tools exits 0; no acme: / @acme/tools / alias: acme remain; .claude/rules/acme.md gone; status --check exits 0 PR pass package remove @acme/tools exit 0; sources:, packages: both lost every acme reference; .claude/rules/acme.md gone; status --check -> all good., exit 0.
13 AC8 source command - source add rules docs/rules --after sync:rules yields order sync:rules, docs/rules, intelligence/rules; source add rules @ainova-systems/sync/rules and source remove rules sync:rules each refused, naming intelligence package PR pass Order matched exactly. Both refusals returned exit 1 and named intelligence package add / intelligence package remove @ainova-systems/sync respectively.
14 AC9 Docs and release - the 0.18.1 changelog section byte-identical to origin/main; the 0.19.0 section holds the lines this PR adds under Added/Fixed, no Breaking item from this PR; docs/cli.md describes the three spellings/alias command/<=0.18 behavior; decisions/0019-* records the design; decisions/0005-* byte-identical to origin/main; intelligence/rules/cli.md differs only in the Who owns what sentence PR pass diff of the extracted 0.18.1 changelog section against origin/main was empty. The 0.19.0 additions are confined to Added/Fixed; git diff origin/main for CHANGELOG.md shows no lines under Breaking. decisions/0019-name-package-sources-by-package.md exists. git diff origin/main for decisions/0005*.md empty. git diff origin/main for intelligence/rules/cli.md shows only the one Who owns what sentence changed. docs/cli.md verified under step 5 above.
15 AC10 Trailing comment - append a trailing comment to the intelligence/rules entry, add a marker file; sync exits 0 and .claude/rules/marker.md exists; status --check exits 0; source list shows the entry as intelligence/rules PR pass sync exit 0, .claude/rules/marker.md present and rendered. status --check -> all good., exit 0 (the commented entry caused no finding). source list showed the entry as intelligence/rules, comment stripped from display.

@dzykovic dzykovic added ai:verified Declared behavior verified on the reported head 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 b1113b0 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.

2 participants