fix(vc-canary): fleet substrate doctrine + strict merge-catalog + legacy units alias - #66
fix(vc-canary): fleet substrate doctrine + strict merge-catalog + legacy units alias#66div0-space wants to merge 9 commits into
Conversation
Loads every language plugin before catalog settlement and fails closed on the first missing required field, invalid kind, or unmatched source file. Adds shell, TypeScript, JavaScript, and TOML contracts with executable regression coverage. Authored-By: codex <agents@vetcoders.io> session_id: 01a02083-f82e-76c1-87ca-b0d79ecbca60 time: 2026-08-20T21:01:36+02:00 runtime: headless
…nces — first live run findings - FLEET: N=1 → Living Tree, N>1 → Fleet Worktrees (Mode B): worktree+commit per scope, single-thread integrator; native subagents inherit the parent model; per-scope scratchpad dir (flat tmp names collided between parallel scopes) - SENSE: scale-adaptive split when one plane dwarfs the rest (73% of LOC in one crate today); deferred scopes declared as waves in scopes.json - FINDINGS: only after the fleet settles — loct follow counts drifted 4→9 mid-wave; pin the snapshot fingerprint next to quoted counts - brief: substrate block replaces the hardcoded Living Tree header; explicit return envelope (top-level key 'catalog'); FENCE for generated / SRI-pinned / vendored artifacts — a comment byte in an SRI-pinned file is an outage, not documentation - canary_cli: legacy top-level 'units' accepted with a loud stderr warning (today's 8-scope run would have died at merge); +1 regression test, 5/5 green on the b568a66 suite Builds on b568a66 (codex: strict plugin enforcement). Evidence: loctree-suite wave 1, 2026-08-20 — 8 scopes, 1299 units, commit 788400c0 in Loctree/loctree-suite. Authored-By: claude <agents@vetcoders.io> session_id: 760d0c62-f5af-49d7-a8a1-09532f6a9584 time: 2026-08-20T21:26:39+02:00 runtime: interactive
There was a problem hiding this comment.
An organization admin can view or raise the cap at claude.ai/admin-settings/claude-code. The cap resets at the start of the next billing period.
Once the cap resets or is raised, reopen this pull request to trigger a review.
There was a problem hiding this comment.
Pull request overview
This PR strengthens vc-canary with strict catalog validation, fleet/worktree guidance, and legacy catalog compatibility.
Changes:
- Adds language-specific plugin contracts and strict merge validation.
- Documents fleet execution, worktrees, scratchpads, and settlement rules.
- Supports the legacy
unitsalias with warnings. - Adds regression and contract tests.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Summary and final comments |
|---|---|
vibecrafted-core/vibecrafted_core/skills/vc-canary/SKILL.md |
Updates canary doctrine. Nit (2 votes): Polish mirror is stale and may fail localization freshness checks. |
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py |
Implements plugin loading and strict validation. Critical (4 votes): Default inventory includes unsupported languages and Makefile, causing normal runs to fail under strict mode; add contracts or restrict the supported inventory. |
vibecrafted-core/vibecrafted_core/skills/vc-canary/references/canary-agent-brief.md |
Defines the fleet brief and catalog envelope. Nits (2 votes each): Update the stale Polish mirror and correct the legacy alias wording to identify catalog as canonical and units as warned compatibility syntax. |
vibecrafted-core/vibecrafted_core/skills/vc-canary/plugins/typescript.py |
Adds the TypeScript contract. Critical (1 vote): Include enum among allowed declaration kinds. |
vibecrafted-core/vibecrafted_core/skills/vc-canary/plugins/toml.py |
Adds the TOML plugin contract. |
vibecrafted-core/vibecrafted_core/skills/vc-canary/plugins/shell.py |
Adds the shell plugin contract. |
vibecrafted-core/vibecrafted_core/skills/vc-canary/plugins/python.py |
Extends Python plugin metadata. |
vibecrafted-core/vibecrafted_core/skills/vc-canary/plugins/javascript.py |
Adds the JavaScript plugin contract. |
vibecrafted-core/vibecrafted_core/skills/vc-canary/FLOW.md |
Documents worktree-aware fleet flow. Nit (2 votes): Polish mirror is stale and may fail localization freshness checks. |
vibecrafted-core/tests/test_canary_cli.py |
Adds catalog validation and alias regression tests. |
Suppressed comments (5)
vibecrafted-core/vibecrafted_core/skills/vc-canary/plugins/javascript.py:4
- The JavaScript contract copies the TypeScript enum and therefore accepts
interfaceandtypefor.js,.jsx,.mjs, and.cjsunits. Those are not ECMAScript declarations (and are especially invalid for the module extensions), so malformed JavaScript catalogs can pass strict validation. Remove these kinds or introduce an explicit Flow contract instead of accepting them here, and add a regression test.
KIND_ENUM = ("function", "class", "interface", "type", "const", "method", "module")
vibecrafted-core/vibecrafted_core/skills/vc-canary/references/canary-agent-brief.md:34
- Fleet mode has no unambiguous way to deliver these catalogs to the integrator: workers are told to work only in
{WORKTREE_PATH}, but the required write uses{ROOT}, and.loctree/is ignored (.gitignore:53). If{ROOT}is the shared integration root, the write violates isolation; if it is the worker worktree, the untracked file is absent when the integrator runsmerge-catalog. Define the handoff/collection step explicitly (for example, have the supervisor copy each returned scope JSON into the integration root) and use an unambiguous path.
Return ONE JSON object — written to
{ROOT}/.loctree/canary/catalogs/{SCOPE_ID}.json AND returned as your final
message. The top-level key is `catalog`; merge-catalog reads no other name:
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:724
- Checking
cat is Nonetreats an explicitly present but invalid canonical key as valid legacy input:{ "catalog": null, "units": [...] }is silently merged, so strict mode does not fail closed on malformedcatalog. Restrict this compatibility branch to the case wherecatalogis absent (and reject or explicitly define the behavior when both keys are present).
if (
cat is None
and isinstance(data, dict)
and isinstance(data.get("units"), list)
):
cat = data["units"]
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:213
REQUIRED_FIELDSis treated as a presence check only. Values such as"authority": "typo","line": [], or"docstring_added": "false"all pass--strict, even though the brief declaresauthorityasrepo_verified|inferredand these fields have typed meanings in the catalog example. Malformed units are then emitted as valid. Add per-field type/enum validation to the plugin contract and cover invalid values.
for field in plugin.required_fields:
value = unit.get(field)
if (
field not in unit
or value is None
or (isinstance(value, str) and not value.strip())
):
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:723
- Strict mode still accepts a bare JSON array because the preceding assignment treats any non-dict as the catalog; only the
unitsalias gets a warning here. That lets a worker bypass the new brief's required top-levelcatalogenvelope while--strictsucceeds. Reject non-object inputs in strict mode and keep any raw-list behavior only behind the explicit compatibility path.
if (
cat is None
and isinstance(data, dict)
and isinstance(data.get("units"), list)
):
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…allback, TS enum, alias truth, PL mirror parity - canary_cli.py: files whose language ships no plugin (swift, make, go, java, …) validate fail-closed against the shared required-field contract with the kind enum waived — strict merge no longer rejects whole languages the atlas inventories by default; 2 new tests (7/7 green) - plugins/typescript.py: 'enum' is a catalogable TS declaration kind - canary-agent-brief.md: 'catalog' is canonical, 'units' a warned legacy alias — the sentence no longer contradicts the compatibility branch - SKILL.md: settle-contract paragraph describes the fallback honestly - pl/vc-canary mirrors synced to the new canonical (this mirror's convention is a verbatim copy): kills the stale 'do not worktree' template the fleet doctrine explicitly retires, satisfies the localization freshness gate Addresses Copilot review threads on #66. Authored-By: claude <agents@vetcoders.io> session_id: 760d0c62-f5af-49d7-a8a1-09532f6a9584 time: 2026-08-22T09:20:06+02:00 runtime: interactive
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Suppressed comments (7)
Previously missed (2) — in code that hasn't changed since the last review.
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:241
- Strict mode is described as enforcing the catalog schema, but this loop only checks that required values are non-empty. The brief fixes
authoritytorepo_verified|inferred; a supported-language unit withauthority: "unverified"(and non-booleandocstring_addedor non-integerline) is accepted. Validate field types/enums as well as presence, including the pluginless branch.
for field in plugin.required_fields:
value = unit.get(field)
if (
field not in unit
or value is None
or (isinstance(value, str) and not value.strip())
):
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:760
- Strict validation runs before
write_json(out, merged), but it does not invalidate an existing output. After one successful merge, a later invalid scope leaves the previous.loctree/canary/catalog.jsonin place even though this invocation failed, so downstream findings can consume stale data as the current merge. Remove/invalidate the old artifact on failure or publish a run-scoped output with an explicit validity marker.
if args.strict:
validate_catalog(p, cat, plugins)
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/FLOW.md:9
- This weakens the substrate rule added in
SKILL.md:93-97: that rule makesN>1the trigger for Fleet Worktrees, not only concurrent execution. As written, a sequential multi-scope run can follow this flow into a shared checkout. Make the condition explicit for everyN>1fleet.
E --> F[Fleet: 1 agent per scope / worktree per scope when parallel]
vibecrafted-core/vibecrafted_core/skills/vc-canary/FLOW.md:9
- This weakens the substrate rule added in
SKILL.md:93-97: that rule makesN>1the trigger for Fleet Worktrees, not only concurrent execution. As written, a sequential multi-scope run can follow this flow into a shared checkout. Make the condition explicit for everyN>1fleet.
E --> F[Fleet: 1 agent per scope / worktree per scope when parallel]
vibecrafted-core/vibecrafted_core/skills/vc-canary/references/canary-agent-brief.md:11
- This Fleet block requires
{WORKTREE_PATH}and{SCOPE_BRANCH}, but no current canary launcher fills them;LIVING_TREE_RULE.md:89-95makes worktrees dispatch-only. A native/interactive N>1 run will either have unresolved placeholders or violate its shared-checkout rules. Emit this block only for a validated Mode B dispatch, or add the launcher/integrator path first.
Fleet (N>1, Fleet Worktrees): work ONLY inside your worktree {WORKTREE_PATH}
on branch {SCOPE_BRANCH}; commit your scope there; never touch the shared
checkout — integration is single-threaded and is not your job.
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:211
- Because
raw_fileis only checked for emptiness and then passed unnormalized, a unit withfile: " src/lib.rs "is treated as pluginless and only receives the shared contract; its invalidkindcan therefore pass strict mode. Normalize the value for plugin resolution or reject surrounding whitespace before allowing the pluginless fallback.
raw_file = unit.get("file")
if not isinstance(raw_file, str) or not raw_file.strip():
_die(f"{location}: missing required field 'file'")
plugin = plugin_for_file(raw_file, plugins)
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:751
- Strict mode still accepts a raw top-level JSON array through the
else datapath immediately above this alias branch. That bypasses the newly documented object envelope (catalog, with onlyunitsas a warned legacy alias) and produces no warning, so non-conforming worker output is silently merged. Reject non-object payloads in strict mode, or explicitly document and test the array as another compatibility format.
if (
cat is None
and isinstance(data, dict)
and isinstance(data.get("units"), list)
):
…onstruction; ROOT is the worker's substrate root
- SKILL.md: name how the canary fleet satisfies Living Tree Rule Mode B
(briefs = written dispatch plan, per-scope gates = pre-committed verifiers,
disjoint domains, session = single-thread integrator) instead of an
unconditional worktree order the rule's letter does not back
- canary-agent-brief.md: {ROOT} defined as the worker's OWN substrate root —
worktree in fleet mode; the integrator collects catalogs at merge, so no
worker writes into the shared checkout
- pl mirrors synced (verbatim-copy convention)
Addresses second-round Copilot threads on #66.
Authored-By: claude <agents@vetcoders.io>
session_id: 760d0c62-f5af-49d7-a8a1-09532f6a9584
time: 2026-08-22T10:04:42+02:00
runtime: interactive
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Suppressed comments (7)
Previously missed (4) — in code that hasn't changed since the last review.
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/SKILL.md:97
- This claims every N>1 run is already a Mode B formation, but the launch paths documented above are ordinary
vibecrafted canary/interactive launches.skills/LIVING_TREE_RULE.md:89-95explicitly says the everyday launcher has no worktree and Mode B is dispatch-only; its verifier, geometry, and integrator requirements are not provided by these briefs alone. Route fleet canary through the dispatch worktree machinery, or qualify this rule as applying only when a verified dispatch plan has supplied the worktrees.
**Substrate is mechanics, not judgement:** N=1 → Living Tree; N>1 → Fleet
Worktrees as a Living Tree Rule **Mode B formation** — canary satisfies its
conditions by construction: the per-scope briefs are the written dispatch
plan, the per-scope gates are the pre-committed verifiers, scope domains are
disjoint, and the canary session is the single-thread integrator. One
vibecrafted-core/vibecrafted_core/skills/vc-canary/SKILL.md:97
- This claims every N>1 run is already a Mode B formation, but the launch paths documented above are ordinary
vibecrafted canary/interactive launches.skills/LIVING_TREE_RULE.md:89-95explicitly says the everyday launcher has no worktree and Mode B is dispatch-only; its verifier, geometry, and integrator requirements are not provided by these briefs alone. Route fleet canary through the dispatch worktree machinery, or qualify this rule as applying only when a verified dispatch plan has supplied the worktrees.
**Substrate is mechanics, not judgement:** N=1 → Living Tree; N>1 → Fleet
Worktrees as a Living Tree Rule **Mode B formation** — canary satisfies its
conditions by construction: the per-scope briefs are the written dispatch
plan, the per-scope gates are the pre-committed verifiers, scope domains are
disjoint, and the canary session is the single-thread integrator. One
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:221
- This presence check accepts malformed non-string values (for example
name=[],line={},authority=0, orauthority="made-up") because onlyNoneand blank strings are rejected. The brief specifies concrete catalog values such asauthority=repo_verified|inferred, so strict mode can still emit invalid units; add shared per-field type/enum validation for the pluginless path too.
value = unit.get(field)
if (
field not in unit
or value is None
or (isinstance(value, str) and not value.strip())
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:760
- If a prior merge succeeded, a later strict validation failure leaves the old
catalog.jsonin place because validation happens before this write but never invalidates the previous artifact. Downstream tooling can then consume a stale catalog while the command reports failure, contradicting the fail-closed settlement contract. Remove/invalidate the prior output or attach and enforce a run fingerprint when validation fails, with a regression test for an existing output.
if args.strict:
validate_catalog(p, cat, plugins)
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/SKILL.md:99
- The fleet path now requires each worker to commit its scope and the integrator to merge branches, but the overview at
SKILL.md:33-36still says canary "commits once" (and the flow still describes one commit). Clarify that fleet integration squash-merges to one commit, or update those statements so the required handoff/commit shape is not contradictory.
worktree per scope branched from the integration base; the agent commits
inside its own worktree; the integrator merges scope branches sequentially.
vibecrafted-core/vibecrafted_core/skills/vc-canary/SKILL.md:99
- The fleet path now requires each worker to commit its scope and the integrator to merge branches, but the overview at
SKILL.md:33-36still says canary "commits once" (and the flow still describes one commit). Clarify that fleet integration squash-merges to one commit, or update those statements so the required handoff/commit shape is not contradictory.
worktree per scope branched from the integration base; the agent commits
inside its own worktree; the integrator merges scope branches sequentially.
vibecrafted-core/vibecrafted_core/skills/vc-canary/plugins/javascript.py:4
javascript.pyis selected for every*.js/*.jsx/*.mjs/*.cjsunit, butinterfaceandtypeare TypeScript declaration kinds, not JavaScript kinds. A malformed JavaScript catalog using either value will therefore pass the new strict enum check. Keep these values in the TypeScript plugin only.
KIND_ENUM = ("function", "class", "interface", "type", "const", "method", "module")
…on disk, empty merge fails closed, whitespace cannot dodge plugins - canary-agent-brief.md: .loctree/ is gitignored so scope-branch merges do not carry catalogs — the integrator reads them from each scope worktree on disk (returned JSON as backstop) before removing any worktree; PL synced - canary_cli.py: merge-catalog dies on an empty input dir and on zero total units — an incomplete canary must not look settled - canary_cli.py: unit file paths are stripped before plugin resolution — 'src/demo.sh ' validates as shell, not as pluginless fallback - 3 new tests (10/10 green) Addresses third-round Copilot threads on #66. Authored-By: claude <agents@vetcoders.io> session_id: 760d0c62-f5af-49d7-a8a1-09532f6a9584 time: 2026-08-22T10:52:01+02:00 runtime: interactive
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Suppressed comments (11)
Previously missed (4) — in code that hasn't changed since the last review.
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/SKILL.md:97
- This makes
N>1sufficient for Fleet Worktrees, but the repository'sLIVING_TREE_RULE.md:43-71requires all four Mode B conditions (including pre-committed, non-fakeable verifiers) andLIVING_TREE_RULE.md:89-95explicitly says Mode B is unreachable from the everydayvibecrafted <skill> <agent>launcher. The canary entry points above do not provision a worktree, branch, or integrator, so parallel canary workers can still land in the shared checkout despite this instruction. Route canary through the dispatch worktree surface or make this rule conditional on an existing Mode B plan.
**Substrate is mechanics, not judgement:** N=1 → Living Tree; N>1 → Fleet
Worktrees as a Living Tree Rule **Mode B formation** — canary satisfies its
conditions by construction: the per-scope briefs are the written dispatch
plan, the per-scope gates are the pre-committed verifiers, scope domains are
disjoint, and the canary session is the single-thread integrator. One
vibecrafted-core/vibecrafted_core/skills/vc-canary/SKILL.md:97
- This makes
N>1sufficient for Fleet Worktrees, but the repository'sLIVING_TREE_RULE.md:43-71requires all four Mode B conditions (including pre-committed, non-fakeable verifiers) andLIVING_TREE_RULE.md:89-95explicitly says Mode B is unreachable from the everydayvibecrafted <skill> <agent>launcher. The canary entry points above do not provision a worktree, branch, or integrator, so parallel canary workers can still land in the shared checkout despite this instruction. Route canary through the dispatch worktree surface or make this rule conditional on an existing Mode B plan.
**Substrate is mechanics, not judgement:** N=1 → Living Tree; N>1 → Fleet
Worktrees as a Living Tree Rule **Mode B formation** — canary satisfies its
conditions by construction: the per-scope briefs are the written dispatch
plan, the per-scope gates are the pre-committed verifiers, scope domains are
disjoint, and the canary session is the single-thread integrator. One
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:775
- Strict validation only checks that
docstring_addedis present and non-null, so a malformed value such as the string"false"passes validation and is truthy here, incrementingdocstrings_added. That lets a supposedly strict merge publish an inaccurate count; validate the field's boolean type (and the other schema enums/types) before aggregation, with the same rule applied to the pluginless branch.
added += sum(1 for u in cat if isinstance(u, dict) and u.get("docstring_added"))
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:242
- The brief documents
authority=repo_verified|inferred, but strict validation only checks that the field is non-empty. A catalog unit with"authority": "invented"therefore passes the new strict mode, so the provenance contract is not actually enforced. Validate this enum in both the plugin and pluginless branches (and add a regression case).
for field in plugin.required_fields:
value = unit.get(field)
if (
field not in unit
or value is None
vibecrafted-core/tests/test_canary_cli.py:19
- The helper loads the CLI under the exact name of this pytest module and then assigns it to
sys.modulesat line 23. When pytest imports this file astest_canary_cli, this replaces the test-module entry; a later import or plugin lookup can receive the CLI module instead. Use a distinct module name (and clean it up if it is registered).
spec = importlib.util.spec_from_file_location("test_canary_cli", CLI)
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/FLOW.md:9
- The flow diagram makes worktrees conditional on “when parallel,” while the new canary contract defines every N>1 formation as Fleet Worktrees. A multi-scope run can therefore follow this diagram without worktrees and violate the substrate rule stated in
SKILL.md:92-100. Keep the diagram aligned with the N>1 contract.
E --> F[Fleet: 1 agent per scope / worktree per scope when parallel]
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/SKILL.md:112
- This new fleet protocol says each worker commits its branch and the integrator merges those branches, but the overview/front matter above still promises a single supervisor commit. That contradiction can make a fleet run follow the solo commit protocol; scope the one-commit statement to solo mode and update the mirrored Polish skill as well.
4. **SETTLE** — strict merge validates every catalog unit against the language
plugin resolved from its `file`; `diff-audit` (**no auto-revert** — examine
why, ask operator); compile/lint via plugin; **one** commit (solo) or a
single-thread integration of scope branches with `diff-audit` re-run on the
integrated tree (fleet).
vibecrafted-core/vibecrafted_core/skills/vc-canary/FLOW.md:9
- The flow diagram makes worktrees conditional on “when parallel,” while the new canary contract defines every N>1 formation as Fleet Worktrees. A multi-scope run can therefore follow this diagram without worktrees and violate the substrate rule stated in
SKILL.md:92-100. Keep the diagram aligned with the N>1 contract.
E --> F[Fleet: 1 agent per scope / worktree per scope when parallel]
vibecrafted-core/vibecrafted_core/skills/vc-canary/SKILL.md:112
- This new fleet protocol says each worker commits its branch and the integrator merges those branches, but the overview/front matter above still promises a single supervisor commit. That contradiction can make a fleet run follow the solo commit protocol; scope the one-commit statement to solo mode and update the mirrored Polish skill as well.
4. **SETTLE** — strict merge validates every catalog unit against the language
plugin resolved from its `file`; `diff-audit` (**no auto-revert** — examine
why, ask operator); compile/lint via plugin; **one** commit (solo) or a
single-thread integration of scope branches with `diff-audit` re-run on the
integrated tree (fleet).
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:754
- The alias condition uses
cat is None, so a file containing an explicitly present but invalid canonical value such as{"catalog": null, "units": [...]}silently falls back tounits. That can merge stale or unintended data while hiding the malformedcatalogfield; accept the legacy alias only when thecatalogkey is absent, otherwise let the canonical value fail validation.
if (
cat is None
and isinstance(data, dict)
and isinstance(data.get("units"), list)
):
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:763
- If a prior merge created
out, any validation failure here (and the empty-merge failures below) leaves that oldcatalog.jsonuntouched. The failed settle can therefore leave a stale artifact that looks like the current merged catalog, despite the documented fail-closed behavior; invalidate/mark the previous output on failure or make consumers verify a run/input fingerprint.
if args.strict:
validate_catalog(p, cat, plugins)
…; fleet handoff is copy-then-merge - canary_cli.py: 'file' with surrounding whitespace is rejected with a named error instead of silently normalized — no unresolvable path can reach the merged catalog and no unit can dodge its language plugin - canary-agent-brief.md (EN+PL): the integrator COPIES per-scope catalogs from every worktree into the integration checkout's catalogs dir and only then runs merge-catalog, which scans exactly one --input-dir — the documented flow is now executable as written - round-3 whitespace test upgraded to assert the hard reject (10/10 green) Addresses fourth-round Copilot threads on #66. Authored-By: claude <agents@vetcoders.io> session_id: 760d0c62-f5af-49d7-a8a1-09532f6a9584 time: 2026-08-22T11:28:27+02:00 runtime: interactive
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
Previously missed (1) — in code that hasn't changed since the last review.
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:247
- Strict mode treats a required field as valid as long as it is non-null and, for strings, nonblank; it never validates the field's semantic type or the
authorityenum. Consequently a unit withdocstring_added: "false"is accepted and counted as an addition by line 779, whileauthority: "made_up"andline: []also pass. Validate these field types/allowed values (or make them part of the plugin contract) before merging.
value = unit.get(field)
if (
field not in unit
or value is None
or (isinstance(value, str) and not value.strip())
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/FLOW.md:9
SKILL.mdnow makes N>1 ⇒ Fleet Worktrees unconditional, but this flow labels the worktree requirement “when parallel.” That wording permits a multi-scope run to keep a shared checkout merely by dispatching agents sequentially, which is the unsafe substrate this doctrine is meant to forbid. Make the worktree-per-scope requirement unconditional here as well.
E --> F[Fleet: 1 agent per scope / worktree per scope when parallel]
vibecrafted-core/vibecrafted_core/skills/vc-canary/FLOW.md:9
SKILL.mdnow makes N>1 ⇒ Fleet Worktrees unconditional, but this flow labels the worktree requirement “when parallel.” That wording permits a multi-scope run to keep a shared checkout merely by dispatching agents sequentially, which is the unsafe substrate this doctrine is meant to forbid. Make the worktree-per-scope requirement unconditional here as well.
E --> F[Fleet: 1 agent per scope / worktree per scope when parallel]
vibecrafted-core/vibecrafted_core/skills/vc-canary/SKILL.md:97
- This says every N>1 run satisfies Living Tree Rule Mode B by construction, but the canonical rule in
vibecrafted_core/skills/LIVING_TREE_RULE.md:41-71also requires delivery verifiers committed before dispatch and the standard provider-neutral worktree/cut/<cut-id>geometry. The new canary brief only supplies placeholders for a worktree and post-run gate results, so a supervisor can follow this text while violating those active Mode B prerequisites. Make those prerequisites explicit/enforced here, or do not claim the formation is satisfied by construction.
**Substrate is mechanics, not judgement:** N=1 → Living Tree; N>1 → Fleet
Worktrees as a Living Tree Rule **Mode B formation** — canary satisfies its
conditions by construction: the per-scope briefs are the written dispatch
plan, the per-scope gates are the pre-committed verifiers, scope domains are
disjoint, and the canary session is the single-thread integrator. One
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:151
- The loader only checks that
REQUIRED_FIELDSis a non-empty tuple; it does not require a plugin contract to include the shared fields. A future supported-language plugin that omitsroleorauthoritywill therefore make strict merge accept units missing those fields, because the shared-field fallback runs only for pluginless files. Validate each plugin's required-field list againstSHARED_REQUIRED_FIELDSbefore registering it.
invalid = [
field
for field, value in values.items()
if not isinstance(value, tuple)
or not value
or not all(isinstance(item, str) and item for item in value)
]
…ooking artifact - cmd_merge_catalog unlinks any existing catalog.json before validation: output exists if and only if THIS merge succeeded — a failed rerun can no longer serve yesterday's catalog as settled - regression test (11/11 green) Addresses fifth-round Copilot thread on #66. Authored-By: claude <agents@vetcoders.io> session_id: 760d0c62-f5af-49d7-a8a1-09532f6a9584 time: 2026-08-22T12:24:26+02:00 runtime: interactive
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Suppressed comments (8)
Previously missed (1) — in code that hasn't changed since the last review.
vibecrafted-core/vibecrafted_core/skills/vc-canary/plugins/javascript.py:4
javascript.pyis being treated as a JavaScript contract, but this enum also accepts the TypeScript-only kindsinterface,type, andenum. A catalog for an ordinary.jsfile can therefore pass strict validation while claiming a declaration kind that JavaScript cannot contain, defeating the language-specific check. Keep the JavaScript enum limited to kinds supported by the JavaScript contract (or explicitly make this a shared/Flow contract).
KIND_ENUM = ("function", "class", "interface", "type", "const", "method", "module")
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/SKILL.md:97
- This localized copy makes the same claim that N>1 canary runs use worktrees "by construction", but the ordinary
vibecrafted canary <agent>path is the everyday workflow launcher;skills/LIVING_TREE_RULE.md:89-95documents that this launcher has no worktree or integrator support. Parallel canary workers can therefore still share and overwrite the checkout unless the operator explicitly enters the dispatch Mode B surface. Please state that requirement here (or add the missing launcher integration) instead of asserting the safety invariant already holds.
**Substrate is mechanics, not judgement:** N=1 → Living Tree; N>1 → Fleet
Worktrees as a Living Tree Rule **Mode B formation** — canary satisfies its
conditions by construction: the per-scope briefs are the written dispatch
plan, the per-scope gates are the pre-committed verifiers, scope domains are
disjoint, and the canary session is the single-thread integrator. One
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/SKILL.md:97
- The phrase "per-scope gates are the pre-committed verifiers" does not match the brief contract:
gate.compile/gate.lintare values the worker reports in its returned JSON, not non-fakeable checks committed on the integration base before dispatch. The repository's Mode B rule requires those verifiers first, so treating this self-report as satisfying the prerequisite can admit an unverifiable fleet. Please require an operator-explicit dispatch with committed verifiers and treat the returned gate only as evidence of the run.
conditions by construction: the per-scope briefs are the written dispatch
plan, the per-scope gates are the pre-committed verifiers, scope domains are
disjoint, and the canary session is the single-thread integrator. One
vibecrafted-core/vibecrafted_core/skills/vc-canary/SKILL.md:97
- This claims N>1 canary runs use worktrees "by construction", but the ordinary
vibecrafted canary <agent>path is the everyday workflow launcher;skills/LIVING_TREE_RULE.md:89-95documents that this launcher has no worktree or integrator support. Parallel canary workers can therefore still share and overwrite the checkout unless the operator explicitly enters the dispatch Mode B surface. Please state that requirement here (or add the missing launcher integration) instead of asserting the safety invariant already holds.
**Substrate is mechanics, not judgement:** N=1 → Living Tree; N>1 → Fleet
Worktrees as a Living Tree Rule **Mode B formation** — canary satisfies its
conditions by construction: the per-scope briefs are the written dispatch
plan, the per-scope gates are the pre-committed verifiers, scope domains are
disjoint, and the canary session is the single-thread integrator. One
vibecrafted-core/vibecrafted_core/skills/vc-canary/SKILL.md:97
- The phrase "per-scope gates are the pre-committed verifiers" does not match the brief contract:
gate.compile/gate.lintare values the worker reports in its returned JSON, not non-fakeable checks committed on the integration base before dispatch. The repository's Mode B rule requires those verifiers first, so treating this self-report as satisfying the prerequisite can admit an unverifiable fleet. Please require an operator-explicit dispatch with committed verifiers and treat the returned gate only as evidence of the run.
conditions by construction: the per-scope briefs are the written dispatch
plan, the per-scope gates are the pre-committed verifiers, scope domains are
disjoint, and the canary session is the single-thread integrator. One
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:248
- The strict check only verifies that required fields are present and non-empty; it does not enforce the documented
authority=repo_verified|inferredcontract (and it likewise accepts wrong types for fields such aslineanddocstring_added). For example,{"authority": "guess"}is merged successfully anddocstring_added: "false"is counted as a docstring addition. Validate the field types and the authority enum in the shared path used by both plugin and pluginless units.
for field in plugin.required_fields:
value = unit.get(field)
if (
field not in unit
or value is None
or (isinstance(value, str) and not value.strip())
):
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:742
- This cleanup runs before the input glob is evaluated. If a caller puts
--outputin the--input-dir(for examplecatalogs/catalog.json), the command unlinks that input catalog and then silently merges the remaining files; with only that file it reports an empty merge. Reject an output path that is one of the input JSON files (or collect/validate the input paths before deleting the old output) before callingunlink.
# A rerun must never leave yesterday's settled-looking output behind a
# failed merge: drop any existing artifact before validation so failure
# states are unambiguous (output exists ⇔ this merge succeeded).
out.unlink(missing_ok=True)
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:742
- The stale-output cleanup happens after the
src.is_dir()guard, so a rerun with a missing input directory exits at line 738 while leaving yesterday'scatalog.jsonin place. That violates the invariant described in the new comment (output exists ⇔ this merge succeeded) and can make a failed canary look settled; remove the prior output before this guard (or otherwise clear it on this failure path).
out.unlink(missing_ok=True)
…repository - validate_catalog_unit rejects absolute paths and any '..' component before plugin resolution — an ownership catalog settles THIS repo only - regression test covering ../ and absolute escapes (12/12 green) Addresses sixth-round Copilot thread on #66. Authored-By: claude <agents@vetcoders.io> session_id: 760d0c62-f5af-49d7-a8a1-09532f6a9584 time: 2026-08-22T13:05:16+02:00 runtime: interactive
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Suppressed comments (8)
Previously missed (1) — in code that hasn't changed since the last review.
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:258
- This only checks that required values are present and non-empty when they are strings; it accepts values such as
authority: "unverified"orauthority: 123. The agent envelope defines authority asrepo_verified|inferred, so strict settlement can still admit malformed provenance and produce an inaccurate catalog. Add shared value/type validation for authority (and the other typed fields) in both the plugin and pluginless paths.
for field in plugin.required_fields:
value = unit.get(field)
if (
field not in unit
or value is None
or (isinstance(value, str) and not value.strip())
):
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/FLOW.md:23
- This is the Polish
skills/plvariant, but the change replaces its localized phase-contract heading and table with English. The companion Polish rule files preserve Polish prose, so this drops localization from the canary flow; keep this section translated or update the locale consistently.
## Phase contract
| Phase | Question | Output |
| -------- | ----------------------------------- | --------------------------- |
| Atlas | Is inventory complete with receipt? | `.loctree/atlas/*` |
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/FLOW.md:9
- This weakens the canonical substrate rule in
SKILL.md: there,N>1unconditionally means Fleet Worktrees, not worktrees only when execution happens in parallel. Keepingwhen parallelpermits a multi-scope run to be interpreted as using the shared checkout, contrary to the new doctrine.
E --> F[Fleet: 1 agent per scope / worktree per scope when parallel]
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/references/canary-agent-brief.md:12
- This localized Fleet template has the same unresolved-substrate problem: no canary path provisions
{WORKTREE_PATH},{SCOPE_BRANCH}, or{SCRATCHPAD_DIR}before the brief is used. An unchanged template cannot keep parallel workers out of the shared checkout. Make Fleet briefs dispatch-produced and fail closed on unresolved placeholders, or limit this contract to the existing dispatch plane.
SUBSTRATE — the supervisor fills exactly one block:
Solo (N=1, Living Tree): work in the shared checkout; do not switch branch,
do not worktree, do not commit, do not stash.
Fleet (N>1, Fleet Worktrees): work ONLY inside your worktree {WORKTREE_PATH}
on branch {SCOPE_BRANCH}; commit your scope there; never touch the shared
checkout — integration is single-threaded and is not your job.
Scratchpad: {SCRATCHPAD_DIR} is yours alone. Never write flat shared filenames
vibecrafted-core/vibecrafted_core/skills/vc-canary/FLOW.md:9
- This weakens the canonical substrate rule in
SKILL.md: there,N>1unconditionally means Fleet Worktrees, not worktrees only when execution happens in parallel. Keepingwhen parallelpermits a multi-scope run to be interpreted as using the shared checkout, contrary to the new doctrine.
E --> F[Fleet: 1 agent per scope / worktree per scope when parallel]
vibecrafted-core/vibecrafted_core/skills/vc-canary/references/canary-agent-brief.md:12
- These Fleet placeholders are mandatory for safe execution, but no canary path renders this template or provisions
{WORKTREE_PATH},{SCOPE_BRANCH}, or{SCRATCHPAD_DIR}; they are only reference text in this tree. If the normalvibecrafted canarylauncher sends this unchanged, the worker has no real isolated checkout and may fall back to the shared tree. Make the brief dispatch-produced and fail closed on unresolved placeholders, or route canary through the existing dispatch substrate.
SUBSTRATE — the supervisor fills exactly one block:
Solo (N=1, Living Tree): work in the shared checkout; do not switch branch,
do not worktree, do not commit, do not stash.
Fleet (N>1, Fleet Worktrees): work ONLY inside your worktree {WORKTREE_PATH}
on branch {SCOPE_BRANCH}; commit your scope there; never touch the shared
checkout — integration is single-threaded and is not your job.
Scratchpad: {SCRATCHPAD_DIR} is yours alone. Never write flat shared filenames
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:752
- The output is removed only after the
src.is_dir()guard. If the input directory is missing and an older merged catalog exists,_diereturns before this line, so a failed run leaves a stale artifact that still looks settled, contradicting theoutput exists ⇔ this merge succeededcontract. Move the unlink before all validation/early exits (or clear it on every failure path).
out.unlink(missing_ok=True)
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:777
- Strict mode still accepts a bare top-level JSON list through the
else databranch, even though the new brief makescatalogcanonical and definesunitsas the only warned legacy alias. A producer can therefore return[...]and bypass the top-level contract without a warning. Require an object withcatalogin strict mode; keep bare-list handling only behind the explicit compatibility escape hatch.
if (
cat is None
and isinstance(data, dict)
and isinstance(data.get("units"), list)
):
cat = data["units"]
print(
f"warning: {p.name} uses legacy top-level key 'units'; the contract key is 'catalog'",
file=sys.stderr,
)
…ict; output cannot eat its input; worker provisions its own worktree - validate_unit_path_integrity extracted and run on EVERY merge — absolute and ../ escapes plus whitespace paths are output integrity, not a language contract, so --no-strict no longer bypasses them - --output inside --input-dir is rejected before the pre-merge cleanup can delete a scope catalog it was about to merge - SKILL.md: the WORKER creates its scope worktree with the brief's substrate block — no launcher provisioning implied; integrator = the canary session, collecting catalogs from worktree disk before cleanup; PL synced - 2 new tests (14/14 green) Addresses seventh-round Copilot threads on #66. Dispatcher's note: the Mode B objection is hereby settled by doctrine — the operator's standing order (2026-08-20) and the substrate block in the brief ARE the dispatch plane. Authored-By: claude <agents@vetcoders.io> session_id: 760d0c62-f5af-49d7-a8a1-09532f6a9584 time: 2026-08-22T13:59:06+02:00 runtime: interactive
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (8)
Previously missed (1) — in code that hasn't changed since the last review.
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:813
- The strict checks above accept any non-empty value for
docstring_added, so a malformed unit with"docstring_added": "false"still reaches this truthiness-based count and is recorded as an addition. Count only the booleanTruehere, and preferably reject non-boolean values in both validation branches so strict merges cannot emit an invalid catalog.
added += sum(1 for u in cat if isinstance(u, dict) and u.get("docstring_added"))
vibecrafted-core/tests/test_canary_cli.py:118
- These tests verify that every plugin exposes the same required-field tuple, but they never exercise the new TypeScript, JavaScript, or TOML
GLOBS/KIND_ENUMthroughmerge-catalog. A typo in any of those contracts would pass this suite while valid units are treated as pluginless or rejected. Add valid/invalid merge cases for each new plugin, for example by parameterizing the existing shell-plugin case.
def test_every_shipped_language_plugin_uses_the_rust_required_field_contract() -> None:
module = _cli_module()
plugins = {plugin.name: plugin for plugin in module.load_language_plugins()}
expected = plugins["rust.py"].required_fields
assert set(plugins) == {
"javascript.py",
"python.py",
"rust.py",
"shell.py",
"toml.py",
"typescript.py",
}
assert all(plugin.required_fields == expected for plugin in plugins.values())
vibecrafted-core/vibecrafted_core/skills/pl/vc-canary/SKILL.md:97
- This makes every
N>1run a Mode B fleet and says the worker creates its own worktree, but the canonical rule requires an explicit dispatch satisfying all four Mode B conditions and assigns standard worktree provisioning to the dispatcher (vibecrafted_core/skills/LIVING_TREE_RULE.md:41-71). The canary launcher does not satisfy those conditions by construction, so this guidance can send the everyday/vc-canarypath into an unsupported substrate. Gate Fleet Worktrees on the canonical dispatch contract and use dispatcher-provisioned worktrees; otherwise remain in Living Tree.
**Substrate is mechanics, not judgement:** N=1 → Living Tree; N>1 → Fleet
Worktrees as a Living Tree Rule **Mode B formation** — canary satisfies its
conditions by construction: the per-scope briefs are the written dispatch
plan, the per-scope gates are the pre-committed verifiers, scope domains are
disjoint, and the canary session is the single-thread integrator. One
vibecrafted-core/vibecrafted_core/skills/vc-canary/SKILL.md:97
- This makes every
N>1run a Mode B fleet and says the worker creates its own worktree, but the canonical rule requires an explicit dispatch satisfying all four Mode B conditions and assigns standard worktree provisioning to the dispatcher (vibecrafted_core/skills/LIVING_TREE_RULE.md:41-71). The canary launcher does not satisfy those conditions by construction, so this guidance can send the everyday/vc-canarypath into an unsupported substrate. Gate Fleet Worktrees on the canonical dispatch contract and use dispatcher-provisioned worktrees; otherwise remain in Living Tree.
**Substrate is mechanics, not judgement:** N=1 → Living Tree; N>1 → Fleet
Worktrees as a Living Tree Rule **Mode B formation** — canary satisfies its
conditions by construction: the per-scope briefs are the written dispatch
plan, the per-scope gates are the pre-committed verifiers, scope domains are
disjoint, and the canary session is the single-thread integrator. One
vibecrafted-core/vibecrafted_core/skills/vc-canary/references/canary-agent-brief.md:11
- The Fleet block repeats a worker-created worktree contract that current Mode B does not support:
LIVING_TREE_RULE.md:60-71makes the dispatcher own standard worktree geometry, and:89-95says the everydayvibecrafted canarylauncher has no worktree/integrator path. This leaves{WORKTREE_PATH}and{SCOPE_BRANCH}unbound for normal canary runs. Update the brief to require a real dispatch plan/provisioned path, or route canary through that dispatcher.
Fleet (N>1, Fleet Worktrees): work ONLY inside your worktree {WORKTREE_PATH}
on branch {SCOPE_BRANCH}; commit your scope there; never touch the shared
checkout — integration is single-threaded and is not your job.
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:762
- Two failure paths bypass the new cleanup invariant:
out == srcis not caught by this containment test, soout.unlink()attempts to unlink the input directory; andsrc.is_dir()is checked before cleanup, so a failed rerun with a missing input directory leaves an oldcatalog.json. Resolve the equality and move cleanup after the containment guard but before the input-directory check.
if out.resolve().parent == src.resolve() or src.resolve() in out.resolve().parents:
_die(
f"--output {out} lives inside --input-dir {src}; the pre-merge "
"cleanup would destroy its own input — pick an output path outside "
"the catalogs dir"
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:767
- Removing the old output before validation only covers validation failures.
write_jsonwrites directly toout(canary_cli.py:482-486), so a crash or disk-full error during serialization can leave a truncated file that still exists and is mistaken for a settled catalog, contrary to the invariant claimed here. Write the merged JSON to a same-directory temporary file and atomically replaceoutonly after the full write succeeds, cleaning up the temporary file on failure.
# A rerun must never leave yesterday's settled-looking output behind a
# failed merge: drop any existing artifact before validation so failure
# states are unambiguous (output exists ⇔ this merge succeeded).
out.unlink(missing_ok=True)
vibecrafted-core/vibecrafted_core/skills/vc-canary/scripts/canary_cli.py:799
- The comment says path integrity applies under
--no-strict, but this loop only validates dict elements. A catalog such as{"catalog": ["not-a-unit"]}is then merged and written with nofile, so the compatibility escape hatch can still emit a malformed catalog. Reject non-object entries and runvalidate_unit_path_integrityfor every item here.
for ui, u in enumerate(cat):
if isinstance(u, dict):
validate_unit_path_integrity(p, ui, u)
Supersedes #65 — that branch was cut from the 4.2.x release train and dragged 33 unrelated commits into the diff (hence its merge conflicts). This branch is a clean rebuild from origin/main carrying ONLY the two doctrine commits, cherry-picked with authorship intact:
canary CLI tests: 5/5 green on this base.