feat: plugin content buckets, ownership carry-through, and install-time plugin selection (2.3.5) - #550
Merged
Merged
Conversation
Contributor
Author
|
Pushed the adversarial-review fixes as 8 commits on top of 29be05d - all 10 ranked findings plus the cleanup set. No version bump; the 2.3.4 CHANGELOG entry is amended where behavior changed. Severe findings
Small findings
Cleanup set
Verification: full smoke+unit (155 files) and integration tiers green; per-WP filtered slices green before each commit; typecheck and |
leandrodamascena
marked this pull request as ready for review
July 11, 2026 10:10
leandrodamascena
requested changes
Jul 11, 2026
leandrodamascena
left a comment
leandrodamascena
left a comment
Contributor
There was a problem hiding this comment.
CHANGES REQUESTED
This finishes the plugin mechanism from #475 and most of it holds up — I ran the selection journey and the compile carry-through and they work. But I found a few things that need fixing before this lands, and one is a real breaking change the PR description hides.
The bundle → plugin rename is a breaking change, and the PR body says the opposite. The description says a "read-side alias is kept" and that existing bundle: plugins "keep composing via the alias." I tested it: the shipped code rejects bundle: outright — aidlc-stage-schema.ts:205 emits bundle: was renamed; write plugin: for ownership, no alias. So any third-party plugin written against #475 (which shipped bundle:) breaks. Worse, I traced it through compose: one bundle: stage file gets copied into the install and then aborts the entire graph compile, not just that stage, with no self-heal (compose.ts:599). The rename itself is fine, but the compat claim is false and the blast radius is unmitigated. Either keep the alias like the body claims, or drop the claim and add a migration note + skip-and-drop the bad stage instead of poisoning the whole compile.
Disabling a plugin doesn't actually remove what it merged. This is the orphan case I flagged on #475, still open. I composed test-pro into a fresh install, then ran select-plugins aidlc to disable it. The plugin's own stages/scopes go away, but its merged produces/sensors/consumes stay welded into the core stage source and the compiled graph. I traced why: handleSelectPlugins writes the selection and runs aidlc-graph compile, but never re-runs the compose hook that applied those contributions — so compile only filters the plugin's own nodes; the merged edits stay. select-plugins looks like it removed the plugin, but the contributions are still there. Either re-run a clean compose on select, or say plainly that disable hides the plugin's own content and keeps its merged contributions.
select-plugins can strand an active workflow. If I disable a plugin whose scope the current workflow is running under, the next /aidlc hard-errors and --doctor stays all green (aidlc-utility.ts:444). Needs a guard that refuses to disable a plugin the active workflow depends on, or a clear recovery path.
Runner name collision. A plugin named aidlc-<x> generates runner dirs identical to core runners (aidlc-runner-gen.ts:88 returns the bare slug for plugin stages, aidlc-<slug> for core, so an aidlc--named plugin lands on the same path), and write clobbers silently, so /aidlc-<slug> can route to the wrong stage. There's no naming guard — a plugin should not be allowed to claim the aidlc- prefix.
Smaller stuff, non-blocking:
- The
listOfsilent-drop I flagged on #475 is still there for produces/sensors — non-4-space YAML entries drop with no log, even though the consumes parser got that log in this PR (compose.ts:654). - The closure guard only covers
consumes; arequires_stageon a disabled core stage loses the ordering edge with no error (aidlc-graph.ts:1462). - A malformed
plugin.jsonstill aborts the whole packager (no try/catch,package.ts:843). - CHANGELOG says the empty-body stage guard is enforced by "the compose guard" — it's test-only, not a runtime check.
- Disabled-plugin
agents/andknowledge/still load (the docs only promise stages/scopes/runners are filtered, so this is a doc-accuracy note). - Version 2.3.4 collides with #545 — second to merge re-bumps.
Good parts I checked: t223 drives real rollback and the closure guard against the shipped code (not string literals), t224 is a real name-coupling probe, and the shipped dist data files are drift-clean. The selection transaction with snapshot/rollback is the right shape.
One noun everywhere: the distribution unit, the ownership tag on stages/contributions, and (in later commits) the selection surface are all "plugin". "bundle" was legacy naming from the extension-mechanism era; a plugin defines exactly one ownership tag in every real case, so the two-word split bought nothing and confused authors. The word "bundle" is deliberately left free for a possible future collection-of- plugins concept. - Schema: plugin: is canonical; bundle: remains a deprecated read-side alias (both keys known). Both present with different values fails validation; the validator returns a normalized clone exposing only plugin. parseStageFrontmatter synthesizes plugin from an alias-only file; emitStageFrontmatter writes plugin: only (and now carries number/name/plugin in FIELD_ORDER so plugin-authored frontmatter round-trips). - Compose hook: contributions read plugin: first, fall back to bundle:, drop-log a conflict. Fragment sentinel markers were already <!-- plugin:... --> and are byte-unchanged. - test-pro, docs 18 + 10, and fixtures author plugin:; t188 pins the alias contract (one bundle:-only fixture still composes; a conflicting pair drop-logs), t62 pins schema accept/reject, t64 pins parse->emit-> parse round-trip for plugin metadata and the alias. No behavioral change for installs authored with bundle:.
A plugin can now ship scope definitions, agent personas, and per-agent
knowledge alongside stages/sensors/tools/contributions. Proven at scale
by a customer pilot (a real plugin shipping 3 scopes, 18 personas, 90
knowledge files) before landing here.
- packager: scopes/agents/knowledge join the plugin projection
contentDirs; walk() is recursive so knowledge/<agent-slug>/ subtrees
project without special cases.
- compose hook: three copyTreeNoClobber calls route the buckets into the
harness scopes/agents/knowledge dirs - additive-only, same no-clobber +
{{HARNESS_DIR}} substitution + collision drop-log as stages. A plugin
ADDS a persona; it never overwrites a core one.
- naming: plugin scope files are <plugin>-<name>.md and agent files
<plugin>-<role>-agent.md - the plugin prefix replaces core's aidlc-
filename prefix, stem == frontmatter name.
- test-pro gains a scope, an agent, and a knowledge dir as living proof;
its content test's agent roster now unions the plugin's own agents/
bucket so a stage naming a plugin-shipped persona validates at author
time (previously only the core roster counted - a plugin could never
satisfy it).
- stage-body-non-empty guard in the plugin content tests AND t188: a
frontmatter-only stage file compiles, routes, and passes every other
check while being behaviorally dead (a real pilot regression shipped
23 such stages); now it fails loudly at author and compose time.
- doc 18 Section 6 rows flip from deferred to implemented for the three
buckets; memory/ stays deferred deliberately (it targets the workspace
method tree where core ships org/team.md - real collision semantics
that warrant their own design pass).
…ls, compose-time runner regen Wires plugin: ownership end to end - previously the schema accepted it and the compiler dropped it, so nothing downstream could act on it. - compile: plugin: carries from stage frontmatter onto the compiled stage-graph.json node (absent on core nodes, so core emission is byte-identical). Invariants: a plugin-owned slug must start with "<plugin>-", and plugin: aidlc is rejected (core-ness = omitting the field). Authored number/name stay on the existing re-seeded path - carrying them is deliberately out of scope here. - runner skills: a plugin-owned stage's runner is named by its own already-prefixed slug (/test-pro-integration); core stages keep aidlc-<slug> byte-identically. Drift detection parses the driven slug from the skill body instead of assuming the aidlc- prefix. - scope runners: the hardcoded FIRST_BATCH constant is replaced by scope frontmatter - runner: true selects a scope into the default batch. The four shipped scopes carry the flag so shipped output is unchanged; FIRST_BATCH remains as a derived compatibility export. Plugin-owned scopes (plugin: in frontmatter) get bare-name runners. - compose hook: after a successful recompile it now regenerates stage runners (and scope runners when the plugin ships scopes/), so a composed plugin stage's slash command appears the moment it composes in - previously routable but not typeable. Failures drop-log; installs without a skills dir (codex-style, skills emitted at package time) skip with a note. New self-heal probe: plugin stage in the graph but missing its runner dir triggers regeneration even when nothing else changed. - t221-plugin-runner-naming pins carry-through, both invariants, the naming rules, and data-driven batch selection; t188 asserts composed runner skills exist and core runners are untouched.
…n loaders The naming conventions the docs present as binding were enforced for sensors only; stages, scopes, and agents silently tolerated mismatches that break path-derived lookups (runtime stage_file paths, knowledge dirs, drift guards) or last-write-win. With plugins now composing stages/scopes/agents into shared install dirs, both hazards became plugin-vs-core collisions - enforcement is load-bearing. - compile: a stage file whose name stem differs from its frontmatter slug fails loudly (file, stem, slug all named). The schema comment that punted this check to "the parser" now points here. - loaders: duplicate scope names and duplicate agent slugs across two files throw at load naming BOTH files (previously silent last-write- wins), mirroring the sensor loader's discipline. New AIDLC_AGENTS_DIR env seam + _resetAgentsForTests, matching the scopes/sensors seams. - doctor: advisory rows for agent/scope files whose stem does not match their declared name (plugin-owned files must match exactly; core scopes accept the aidlc- filename prefix). Advisory, not hard-fail: a mismatch is recoverable misconfiguration. - plugin content tests gain stem==name assertions so plugin authors catch it before compose; t222-naming-enforcement pins the compile error, both duplicate guards, the env seam, and the doctor advisory.
…s the contract Two generated-surface cleanups that unblock install-time plugin selection: every surface that enumerates stages must derive from the compiled graph, because selection filters that graph. - stage-table: new utility verb mirroring scope-table - renders the SKILL.md Stage Graph table from compiled stage-graph.json inside BEGIN/END markers, --check byte-compares for drift. All four harness SKILL.md copies now carry the generated region. The hand-maintained table had already drifted in two Mode cells (reverse-engineering and code-generation carried prose annotations the compiled graph does not have; the agent names they duplicated are already in the Lead/Support columns) - the generator is authoritative. - compose hook refreshes both generated regions (scope-table + stage-table) after a successful recompile, so composed plugin stages appear in the table the moment they compose in; missing SKILL.md or marker skips with an advisory drop. t188 asserts test-pro's stages land in the composed region. - t32 repointed: bidirectional hand-sync check becomes stage-table --check drift guard + a coverage assertion that the rendered table names every compiled stage exactly once. - state-template.md now documents the state-file section/field contract only (headings, field bullets, checkbox legend, one placeholder row); the engine is the sole source of stage enumeration - the template's hand-enumerated stage list was dead data that had drifted. state-init and the reference/troubleshooting docs point at the compiled grid and the --doctor resync path instead of the template.
…transactional regen Plugins add, the install selects. An install can now choose which plugins' content its users see: a plugins list in tools/data/ harness.json filters the graph compile to enabled plugins (core is the implicit "aidlc" plugin; the three initialization stages are always included), and every generated surface (runner skills, scope grid, SKILL.md stage/scope tables, state rows, doctor) follows the filtered graph automatically. A test-pro-only install shows only /test-pro-* commands and the test-pro scope while core remains installed and re-enableable. An absent plugins key means everything is enabled, so existing installs are unaffected and the shipped dist is byte-identical. - select-plugins utility verb: validates requested names against the known plugin set, writes the selection, recompiles, regenerates runners (pruning newly-disabled ones) and both SKILL.md generated regions - one step, TRANSACTIONAL: harness.json + stage-graph.json + scope-grid.json snapshot before mutating; any late-step failure restores all three and re-runs the regeneration chain against the restored selection, with a loud recovery message if that also fails. - compiled graph persists the FULL stage set; disabled nodes carry enabled: false (key omitted when enabled). loadStageGraph() filters for all runtime consumers; loadStageGraphAll() serves doctor and selection tooling. Stage numbers are seeded over the full graph, so a disable/re-enable round trip is number-stable (pinned byte-for-byte). - closure guard: a selection leaving an enabled stage consuming a required artifact whose only producers are disabled fails compile naming consumer, artifact, producers, and the plugin(s) to enable. - scope fallbacks degrade gracefully under a core-disabled selection: sole-enabled-plugin installs fall back to that plugin's first scope; multi-plugin selections keep the hard error. - compose does not auto-enable: composing a plugin into an install with an active selection that excludes it lands the files and records an advisory drop naming the select-plugins command; the self-heal probes respect enabled flags so a disabled stage is not a compile trigger. - doctor: enabled plugins + per-plugin stage counts; hard-fails on selection/graph disagreement and on torn-run leftovers, naming the recovery command. - test-pro's cross-plugin consumes edges relax to required: false so a test-pro-only selection satisfies closure (its stage bodies already describe those inputs as use-when-present). - t223-plugin-selection: full journey over the shipped test-pro plugin - select, verify surfaces, re-enable with byte-identical numbers, prune, unknown-name error, rollback on late-step failure, closure-guard message; t188 gains the compose-not-enabled advisory case.
The engine's last behavior keyed on hardcoded core scope NAMES was the walking-skeleton stance: SKELETON_ON_SCOPES, a six-name code constant a plugin scope could never join without a code edit. The stance now resolves through the active scope's own file. - new optional scope frontmatter skeleton: on|off, read through loadScopeMetadata; absent = off (composed scopes reshape an existing plan and must not conjure a skeleton Bolt; plugin scopes opt in explicitly). Invalid values throw naming the file. All nine core scopes declare the field explicitly - the same six resolve on as the deleted constant, pinned by test. - env-scope fallback ADOPTED, not just validated: resolve-env-scope and orchestrate's resolveScope previously let an AWS_AIDLC_DEFAULT_SCOPE naming a disabled/unknown scope through to a generic unknown-scope death; both now route through selectionAwareDefaultScope, so a plugin-only install with the shipped core default starts from plain /aidlc. selectionAwareDefaultScope itself now keys off scope OWNERSHIP (core vs plugin) rather than owner-bucket count, fixing a wrong fallback when core scopes were enabled but the preferred one disabled. - prose decoupled: conductor.md, org.md, rules-reading.md, and the four SKILL.md copies describe the ceremony via the skeleton: field instead of name lists; stage-protocol.md's hardcoded per-scope stage-count table is replaced by a pointer at the compiled grid (scope-table). - t224-scope-name-decoupling: a static probe (comment-stripped source scan) asserts no core tool regrows a 3+ core-scope-name literal; fixture tests pin on/off/absent/invalid parsing; the env fallback is pinned under a plugin-only selection.
…ion + help-text harness seam
Three tier reds, two defects:
- The new Enabled-stage-compile-coverage doctor check hard-failed on ANY
uncompiled stage file, breaking the pre-existing contract that an
uncompiled stage without a selection is a deliberate authoring state
surfaced as an advisory (t184's pin). The hard-fail is now gated on an
active selection - there a missing node means a torn select-plugins
run; without one, the pre-existing advisory row owns the case.
- The select-plugins help-text example hardcoded .claude/tools instead
of the ${harnessDir()} seam (t153 in core, t150 after packaging
propagated it to the codex tree verbatim).
…pe-slug fixture route Four integration reds after the branch's feature commits, all adaptation gaps rather than engine defects: - t34: WP7's prose decoupling deleted the per-scope stage-count table wholesale, but t34's contract (from the original .sh) is that the depth section lists every scope. Restore a scope->default-depth table (mirroring the scope files' depth: frontmatter - names and depths only, no stage counts, which stay banned in this protocol). - t66: the designer-export golden fixture predates the runner/skeleton scope frontmatter; regenerate it (delta is exactly the new fields on the scopes block). - t-custom-harness-compile E5: the duplicate-slug fixture wrote the dupe under a mismatched filename, which now trips the new stem==slug guard before the duplicate-slug guard it pins. Route the dupe through a different phase dir with the slug as its stem so the intended guard fires. - t78 red was a parallel-load flake (5s timeout): green alone, no change.
Consolidated entry for the plugin-selection feature set: plugin content buckets (scopes/agents/knowledge), plugin ownership through compile, namespaced runners, install-time selection with transactional regeneration and the closure guard, generated stage table, state contract, skeleton frontmatter decoupling, and the bundle->plugin rename. t68 pins version/changelog/badge agreement.
The bundle->plugin rename originally kept bundle: as a deprecated read-side alias in three layers (parser synthesis, schema normalization, compose fallback), each with slightly different semantics. The alias is gone: one word, one key, everywhere. - schema: a stage carrying bundle: fails validation with 'bundle: was renamed; write plugin: for ownership' - a targeted error naming the fix, not a generic unknown-key. - parser: back to a dumb extractor; no alias synthesis. The schema is the single normalization boundary. - compose hook: a contribution carrying bundle: (alone or beside plugin:) is skipped with a drop naming the rename - a stale plugin tree fails visibly instead of composing under wrong or ambiguous ownership. - t62/t64/t188 flip from alias-accepted pins to rejected-with-named-fix pins; docs and CHANGELOG drop the alias language. 'bundle' remains reserved for a possible future collection-of-plugins concept and is otherwise absent from the mechanism.
Composing a plugin that ships plugin:-keyed stages/scopes/agents onto an installed engine whose schema predates the plugin: ownership key broke that install's graph compile permanently (the retry marker re-failed every SessionStart). compose now probes the INSTALLED schema for plugin: acceptance up front; on rejection it skips the plugin-owned content copy with a degraded drop naming the remediation and does not write the retry marker for that cause (retrying cannot fix an old engine). knowledge/, sensors/, and tools/ still copy. t188 gains an old-engine fixture proving no copy, the drop, no marker, and a still-green compile.
copyTreeNoClobber was filename-only: a plugin agents/scopes file with a unique filename but a frontmatter name duplicating an installed agent/scope landed cleanly, then the duplicate-name throws in loadAgents and loadScopeMetadataAll bricked compile, orchestrate, help, and the statusline on every prompt with no drop attributing the plugin. compose now parses each agents/scopes file's frontmatter name and checks it against the installed roster before copying; a collision skips that file with a degraded drop naming the plugin, the file, and the colliding installed file. The statusline's agentDisplayMap additionally degrades to slug display when loadAgents throws (hooks fail open by design). t188 gains a synthetic collision fixture proving skip + drop + compile green + statusline exit 0; t61 gains a duplicate-name slug-fallback case.
A composed scope (runtime-approved user state: a scopes/aidlc-<name>.md with no plugin: field plus a hand-appended grid entry no stage frontmatter produces) was destroyed by plugin selection twice over: filterScopeGrid stripped it from the written grid when core was deselected (unrecoverable - the transpose cannot re-derive it), and even with core enabled the transpose seeded its name from enabledScopeNames and fabricated a flattened init-only twin that shadowed the real on-disk entry in mergeComposedScopes. The grid pipeline now derives the composed set (on-disk grid keys with no stage frontmatter producer over the FULL stage list), excludes them from transpose seeding, and passes them to filterScopeGrid as an explicit exempt set - a composed entry round-trips byte-identical through any select-plugins run. discoverScopes keeps a composed scope's runner via written-grid membership so the select-plugins regen chain no longer prunes it. t223 gains a composed-scope journey: seed, select test-pro only, re-enable both, byte-identical grid entry each time, runner intact, and next --scope routes a run-stage directive for the real first EXECUTE stage.
Three runner-gen hazards closed. (1) Both prune sites deleted ANY skills dir whose SKILL.md matched the command signature - a user-authored skill that documents those commands was rm -rf'd silently. Generated runners now carry a generated-by: aidlc-runner-gen frontmatter marker; pruning requires signature AND marker, with a one-release legacy rule for pre-marker installs (dir named exactly aidlc-<slug>, or the bare slug when it is plugin-owned). A signature-matching dir that is neither is left alone and listed on stderr as unmanaged. (2) An empty scope batch pruned every scope runner and exited 0 success-shaped, indistinguishable from a stale-scopes upgrade or a mispointed AIDLC_SCOPES_DIR; prune-on-empty now runs only under an active selection, otherwise it warns naming the two likely causes and preserves runners; discoverScopes warns on an unreadable scope dir. (3) The eager FIRST_BATCH module const ran scope-dir I/O at import and threw on one malformed scope file, killing even write; deleted - importers (t123 twins, t130, codex emit) call defaultScopeBatch() at use time, and a malformed scope file no longer kills write. t221 gains six cases pinning all three.
… covers headers Four small hardening fixes. select-plugins no longer errors on a Codex-style install with no harness skills/ dir: regenerateSelectionSurfaces skips the two runner-gen spawns with a note naming the skipped surface and the real path (mirrors compose's advisory; never mkdirs). The AWS_AIDLC_DEFAULT_SCOPE sole-plugin fallback is no longer silent: the lib result carries a note field and both env call sites print it to stderr naming the substituted scope; exit code and stdout unchanged. Doctor's enabled-stage-coverage walk no longer skips a stage file whose frontmatter fails to parse when a selection is active - it cannot be proven in the graph, so it counts as missingEnabled with the parse error in the fix message; the two issue-number references in doctor comments are rewritten as prose contracts. t223/t224 covers: headers rewritten in the registry's class:id grammar; the regenerated coverage registry flips subcommand aidlc-utility select-plugins, function:selectionAwareDefaultScope, function:pluginsEnabled, and function:mergeComposedScopes to covered.
…rammar, fixtures hoist, doctor dedup, selection audit event Six review cleanups. The stage-table/scope-table --check twins collapse into one checkGeneratedTableRegion helper, and replaceGeneratedRegion now shares the same marker-locating core (findGeneratedRegion) so all three surfaces validate duplicate/out-of-order markers identically. The stage-enabled-by-selection predicate is exported once from aidlc-lib (stageEnabledBySelection) and used by graph compile and doctor; compose keeps its self-contained copy with a sync comment. The scope-table BEGIN marker drops its em dash for the hyphen grammar the stage-table already uses (constants, compose copies, all four harness SKILL.md files; an installed SKILL.md still carrying the old marker degrades to compose's existing missing-BEGIN advisory drop, no error). The triplicated test scaffolding hoists into fixtures.ts withEnvAndFreshCaches (delete-aware env restore + all five cache resets) with t221/t222/t223/t224 converted. Doctor reuses a parameterized artifactsRegistryFor and a shared frontmatterBlock helper instead of hand-rolled twins. select-plugins' set-mode emits a PLUGIN_SELECTION_CHANGED audit event (previous + new selection) - registered in VALID_EVENT_TYPES, documented in the state- machine chapter and audit-format registry, pinned by the count tests (70 -> 71) and asserted end-to-end in t223.
…ntry The compose skew guard, the frontmatter-name collision guard, the runner provenance marker + empty-batch gate, composed-scope selection survival, the PLUGIN_SELECTION_CHANGED audit event, the env-fallback stderr note, the doctor parse-fail coverage rule, the Codex skills-dir note, and the scope-table marker grammar change are all user-visible amendments to the same unreleased 2.3.4 - no new version.
The withEnvAndFreshCaches hoist gave fixtures.ts module-load-time imports of core/tools/aidlc-graph.ts and aidlc-lib.ts. t52's drift meta-test runs t48 inside a sandbox copy that carries dist + docs + tests but no core/, so importing fixtures.ts there failed module resolution and broke all six t52 cases. The resets now load the two modules with createRequire at call time (no sandbox test calls them); resolution identity with the tests' own ESM imports verified behaviorally via the t221/t222/t223/t224 consumers.
PR #545 took 2.3.4 and the t221 slot on v2. Version trio re-bumped to 2.3.5 (aidlc-version.ts + CHANGELOG heading + README badge); the four plugin-selection tests re-slot to t222-t225 (t221 is the reviewer-scope hook's); audit-event pins merge both sides' additions (72 events, 19 categories: REVIEWER_SCOPE_BLOCKED from #545 + PLUGIN_SELECTION_CHANGED from this branch); coverage registry + ratchet regenerated; dist regen in sync.
…egex The array-literal probe's regex form backtracked past the 5s test timeout on the rebased aidlc-lib.ts and, worse, could silently skip literals when a match starting at an earlier bracket spanned them inside a quoted-string alternative. A linear bracket-stack scanner is O(n) and found one real coupling the regex missed: the workspace-detection greenfield advisory's three incremental scope names. That advisory predates the probe, so it is exempted by exact signature; any new literal (or that one growing) still fails.
Graph compile is all-or-nothing: aidlc-graph.ts throws on the first schema-invalid stage file, so one bad plugin stage copied into the install (e.g. a stale tree still authoring the renamed bundle: key) bricked every later compile of the whole install with no self-heal. The compose stage copy now validates each plugin stage file against the INSTALLED engine's parser + validator before it lands: an invalid file (or a frontmatter-only empty body) is skip-and-dropped with the file and errors named, the rest of the plugin composes normally, and the self-heal probe excludes deliberately-dropped slugs so it doesn't force a futile recompile every session. Fails open when the installed lib can't be loaded (a partial install already can't compile).
runnerDirName returns the bare slug for plugin stages but aidlc-<slug> for core, so a plugin named aidlc-<x> generated runner dirs identical to core runner paths and runner-gen write clobbered them silently - /aidlc-<slug> could route to the wrong stage. Four chokepoints now refuse the name: graph compile rejects plugin: aidlc-* stage frontmatter (the install-path enforcement point), the compose stage precheck mirrors the same invariants so a bad stage file drops instead of bricking compile, loadScopeMetadataAll rejects aidlc-* plugin: in scope frontmatter (covers scope-runner dirs), and the packager refuses an aidlc/aidlc-* plugin directory at discover + plugin-build time.
…s a stranded one Disabling a plugin an active workflow depends on hard-errored every later /aidlc on that workflow (the state file's scope out-ranks --scope, so 'Unknown scope' had no in-band recovery) while doctor stayed green. select-plugins set-mode now enumerates every non-complete workflow across all spaces and refuses when the new selection would disable (a) the plugin owning the workflow's scope or (b) a plugin owning a pending EXECUTE stage in its plan, naming each dependency and the remediation (complete or park the workflow, or keep the plugin). Doctor gains a matching row that fails when the CURRENT selection (pre-guard or hand-edited) already strands one.
… stages select-plugins disable removed a plugin's own stages/scopes/runners but left its merged produces/sensors/consumes/required_sections and spliced prose welded into CORE stage source and the compiled graph - a disabled plugin kept steering enabled stages. Compose now records what it ACTUALLY added per target stage in a per-plugin sidecar (tools/data/plugin-contrib-<key>.json; actually-added entries only, so removal can never strip a value core already had), and select-plugins strips on disable: structural adds via the sidecar, prose fragments via their existing sentinel markers, with stripped stage files joining the transaction's rollback snapshots. Re-enabling restores everything on the next session start (the plugin's compose hook re-merges; round-trip is byte-identical). Compose also stops merging contributions for a plugin the current selection disables - stage copies stay (runtime-filtered), but contribution merges into unfiltered core source would have undone the disable-time strip every session.
…ring-edge advisory, packager manifest error, changelog + doc accuracy Four smaller review items plus the doc updates the bigger fixes need: - adds.produces/sensors entries that fail the 4-space parse now drop-log a parsed-N-of-M shortfall naming the indentation rule, mirroring the consumes parser (they truncated silently). - An enabled stage whose requires_stage names a selection-disabled stage is surfaced by doctor as an advisory listing each dropped ordering edge (not a closure error: the edge is vacuous when the dep never runs, and plugin-only installs legitimately order plugin stages after core ones). - A malformed plugins/<name>/.aidlc-plugin/plugin.json fails the packager with the plugin and file named instead of a raw JSON.parse stack. - CHANGELOG: the empty-body stage guard is now accurately described as the compose stage-file precheck (it was claimed as a runtime compose guard while test-only); the entry also gains bullets for the contribution strip, the strand guard, and the aidlc- name reservation. Doc 18 documents strip/restore semantics, the not-filtered agents/knowledge surfaces, the dropped-edge advisory, and the reserved names; the CLI guide documents the strip + refusal.
apackeer
force-pushed
the
feature/plugin-selection
branch
from
July 12, 2026 20:06
6c3fd8a to
91a7108
Compare
apackeer
changed the title
Contributor
Author
|
Thanks for the thorough pass - all four blockers were real, and all four are fixed, plus the non-blocking set. Rebased onto v2@2.3.4 first (#545 took 2.3.4 and the t221 slot), so this branch is now 2.3.5 with tests re-slotted to t222-t225. 1. 2. Disable now removes what the plugin merged. The orphan case is closed for structural adds AND prose. Compose records what it actually added per target stage (never what was declared - so removal can never strip a value core already had) in a per-plugin sidecar at 3. Strand guard. 4. Non-blocking items, all addressed:
Full deterministic tiers green at the new tip; live disable-strip/re-enable drive green on a fresh temp install. PR body updated to match reality. |
A newer bun adds a configVersion field to the lockfile on install, leaving every fresh checkout permanently dirty on git status. One-line metadata; no dependency changes.
Contributor
Author
|
@leandrodamascena all four blockers plus the non-blocking set are fixed and replied to point-by-point above (branch rebased to 2.3.5, tiers green at the tip). When you get a chance, could you re-review? The one open judgment call is the requires_stage-onto-disabled-stage tradeoff (doctor advisory vs hard error) - happy to escalate it if you disagree. |
leandrodamascena
approved these changes
Jul 13, 2026
leandrodamascena
left a comment
leandrodamascena
left a comment
Contributor
There was a problem hiding this comment.
Thanks for addressing the feedback @apackeer! I ran another round of review and couldn't find any bug.
apackeer
added a commit
that referenced
this pull request
Jul 14, 2026
The four-PR train (#550 2.3.5, #562 2.3.6, #563 2.3.7) took this branch's old version and test slots, so versions shift to 2.3.8 (workspace verbs) and 2.3.9 (config/plugin verbs), t230-handler-additions moves to t231, and t231-build-binaries moves to t238 (t232-t237 are claimed by merged and in-flight work). Coverage registry regenerated; dist regen in sync.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat: plugin content buckets, ownership carry-through, and install-time plugin selection (2.3.5)
Completes the plugin mechanism (#475) with the surface a real plugin needs, proven at scale in a customer pilot before landing here: a pilot plugin shipping 23 stages, 18 agent personas, 90 knowledge files, and 4 scopes runs on this exact mechanism.
Design principle: plugins add, the install selects. A plugin never removes or overrides core; an install chooses which plugins' content its users see. Core itself is the implicit
aidlcplugin and can be deselected - a plugin-only install shows only that plugin's commands and scopes while core stays installed and re-enableable.What's in here
bundle:renamed toplugin:- breaking for plugin trees authored against feat: AIDLC plugin mechanism (2.3.0) #475: the old key is rejected everywhere it is read (schema validation errors with the fix named; the compose hook drop-skips abundle:contribution; abundle:-keyed plugin STAGE file is skip-and-dropped at compose copy time instead of landing and bricking the install's graph compile). There is no read-side alias. Migration: renamebundle:toplugin:in your plugin tree. "bundle" is reserved for a possible future collection-of-plugins concept and deliberately unused (see doc 18).scopes/,agents/,knowledge/alongside stages/sensors/tools/contributions - projected by the packager, composed additively (no-clobber, collision drop-logged).memory/stays deferred (real collision semantics vs the workspace method tree).plugin:carries onto compiled stage-graph.json nodes (absent on core nodes - shipped dist byte-identical). Invariants: plugin-owned slugs carry the plugin prefix;plugin: aidlcrejected; plugin names must not start withaidlc-(reserved for core - anaidlc-<x>plugin's runner dirs would land on core runner paths and silently clobber them; enforced at compile, compose, scope loading, and the packager); filename stem == slug enforced at compile; duplicate scope/agent names fail loudly naming both files./test-pro-integration); core keepsaidlc-<slug>byte-identically. Scope-runner batch is data (runner: truefrontmatter), not a code constant. Compose regenerates runners + the SKILL.md generated tables, so composed stages are typeable and visible immediately.select-plugins: transactional (snapshot + rollback - no torn installs), number-stable across round trips, closure-guarded (a selection stranding an enabled consumer of a disabled producer fails compile naming the plugin to enable), doctor-integrated (selection rows, hard-fail on selection/graph disagreement).tools/data/plugin-contrib-<key>.json);select-pluginsstrips a disabled plugin's merged produces/sensors/consumes/required_sections (sidecar) and spliced prose (fragment sentinels) from installed stage source, inside the same rollback transaction. Re-enable + next session start restores everything byte-identically (the plugin's compose hook re-merges). Compose no longer merges contributions for a plugin the current selection disables.select-pluginsrefuses a change that would strand a running workflow (its scope, or a pending EXECUTE stage in its plan, owned by a plugin the new selection disables), naming each dependency and the remediation. Doctor hard-fails on a selection that already strands one, and lists selection-droppedrequires_stageordering edges as an advisory.stage-tableverb, markers,--checkdrift guard; the hand-maintained table had already drifted in 2 cells);state-template.mddocuments the contract instead of hand-enumerating stages. Both follow the filtered graph under a selection.skeleton: on|offscope frontmatter replaces the hardcoded six-name set - plugin scopes are first-class in the ceremony.AWS_AIDLC_DEFAULT_SCOPEnaming a disabled scope falls back to the sole enabled plugin's first scope (a plugin-only install starts from plain/aidlc). A static probe (t225) prevents the name-coupling from regrowing.bundle:key, unknown fields, or a frontmatter-only empty body) is skip-and-dropped naming the file and the errors, the rest of the plugin composes, and the install's graph compile stays green (previously one bad copy bricked every later compile with no self-heal).Compatibility
bundle:frontmatter in third-party plugin trees no longer composes - rename the key toplugin:. The failure is safe (drop-logged, surfaced by/aidlc --doctor, install keeps compiling) but the plugin's content will not land until migrated.Test evidence
Version note
Claims 2.3.5 on top of v2@2.3.4 (rebased after #545 merged and took 2.3.4 + the t221 test slot; this branch's tests re-slotted to t222-t225).