Skip to content

Reduce the client skill surface to three; always activate the graph - #720

Merged
bgmcmullen merged 3 commits into
masterfrom
skills-surface-reduction
Aug 12, 2026
Merged

Reduce the client skill surface to three; always activate the graph#720
bgmcmullen merged 3 commits into
masterfrom
skills-surface-reduction

Conversation

@bgmcmullen

@bgmcmullen bgmcmullen commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Six shipped skills become three on a default install (hypaware-query, hypaware-reference, hypaware-privacy). Three independent reductions land together, each recorded as an Accepted decision, plus the CLI help capability they all lean on.

What changed

LLP 0212 - the session opt-out is a CLI verb. hypaware-ignore and hypaware-unignore were written before hyp session ignore existed, so each carried its own curl against the gateway control route. Once LLP 0067 shipped the verb, the skills became a second, untested implementation, and drifted: a stale http://127.0.0.1:8787 fallback (the real default is 127.0.0.1:18521) and a missing LLP 0066 R9 caveat. Both skills are removed; the CLI verb is the only implementation, and the natural-language routing moves into hypaware-reference.

LLP 0213 - the graph plugin is always active. @hypaware/context-graph and @hypaware/ai-gateway-graph are in the bundled allowlist, so they ship everywhere, but day-to-day dispatch boots bootProfile: 'config' and the walkthrough composes plugins[] from picker rows only. An engine plugin that is neither a client nor an export sink had no slot, so it was never written, and hyp graph --help reported the plugin as unavailable on installs months old. A new compose_with manifest field lets a plugin be pulled in alongside another (here, @hypaware/ai-gateway). With the packaging constraint gone, hypaware-graph merges into hypaware-query as LLP 0196 intended, with the conditional GitHub bridge split out to github.md.

LLP 0216 - reports generate server-side. Generating a report is analysis over a fleet's whole recorded history, which the server is better placed to do. hypaware-report (a router SKILL.md plus seven stage files, in both client trees) is removed. hyp report stays, and the eleven load-bearing constraints the skill was the only home of are handed to the server along with the harm statements that justified them.

LLP 0214 - verbs and plugin groups carry long help. The precondition for moving mechanical prose out of skills. VerbRegistration gains an optional help that verbToCommand passes through to dispatch's existing --help interception, and CommandRegistry gains registerGroup/getGroup so a plugin-owned namespace like graph renders a header and paragraph instead of a bare subcommand table. Registering a group is metadata only: it adds nothing to list(), so it cannot shadow a command or appear as its own subcommand.

Testing

npm test - 3894 pass, 2 skipped. Two failures remain, both from stale node_modules in this checkout rather than from the diff:

  • report-render.test.js / report-render-hrefs.test.js: Cannot find package 'marked' (declared in package.json, not installed).
  • format-parquet-clustering.test.js: installed hyparquet-writer is 0.16.1, package.json pins 0.16.6.

llp-ref-hygiene.test.js passes. It reads git ls-files, so it fails against unstaged deletions and is green once the removals are staged.

New coverage: test/core/group-and-verb-help.test.js, test/core/manifest-compose-with.test.js, test/core/compose-picker-config.test.js, and additions to test/plugins/context-graph-query.test.js.

🤖 Generated with Claude Code

Six shipped skills become three (`hypaware-query`, `hypaware-reference`,
`hypaware-privacy`) via three separate reductions, each recorded as a
decision:

- LLP 0212: `hypaware-ignore` / `hypaware-unignore` retire. They predated
  `hyp session ignore` and carried their own `curl` against the gateway,
  which drifted (stale `127.0.0.1:8787` fallback, a missing R9 caveat).
  The CLI verb is now the only implementation; the natural-language
  routing moves into `hypaware-reference`.
- LLP 0213: `@hypaware/context-graph` and `@hypaware/ai-gateway-graph`
  ship in every install but activated in almost none, for no recorded
  reason. A new `compose_with` manifest field lets the walkthrough pull
  them in alongside `@hypaware/ai-gateway`, which removes the packaging
  constraint that blocked merging `hypaware-graph` into `hypaware-query`
  (GitHub enrichment splits out to `github.md`).
- LLP 0216: report generation moves server-side, so `hypaware-report`
  (eight Markdown files across both client trees) is removed. `hyp report`
  stays; the eleven load-bearing constraints the skill was the only home
  of are handed to the server with their harm statements.

LLP 0214 supplies the help surface those reductions lean on: verbs now
carry a `help` string through `verbToCommand`, and `CommandRegistry`
grows `registerGroup`/`getGroup` so a plugin-owned namespace like `graph`
renders prose instead of a bare subcommand table.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bgmcmullen bgmcmullen added the neutral:adopt Foreign PR adopted into neutral's reconcile scope label Aug 12, 2026
@philcunliffe philcunliffe added the neutral:adopted Adoption completion record: merged while carrying neutral:adopt (LLP 0031) label Aug 12, 2026
…bility

Ten fixes from the round 1 review of #720.

Code (LLP 0213 `compose_with`):

- A rider no longer loses a user's `enabled: false`. A rider has no picker
  row by design, so that flag is its owner's only way to decline it, and
  `mergePlugin`'s pick-implies-enabled rule was deleting it on every
  reconfigure. `mergePlugin` now takes `isRider` and returns early.
- `compose_with` can no longer cross the default-activation boundary.
  `loadPickerCatalog` filters riders through the new `ridersInDefaultSet`,
  so a plugin in `V1_EXCLUDED_FROM_DEFAULT` (an API-backed embedder, a
  credential holder) cannot compose itself in unasked by declaring the field.
- `validateManifest` rejects a `compose_with` naming its own plugin. Such a
  rider is never composable and the fixpoint terminates silently, so the
  plugin would simply be absent everywhere with nothing to read.

Graph availability (`hyp init claude-and-otel-local` wrote a brand new
gateway config with no graph, while the skill beside it said otherwise):

- The preset composes `@hypaware/context-graph` and
  `@hypaware/ai-gateway-graph`. `compose_with` is read in
  `composePickerConfig` alone, so a literal preset must name them itself.
- Both `hypaware-query/SKILL.md` copies regain one diagnostic paragraph:
  if `hyp query status` lists no `node`/`edge`, answer from
  `ai_gateway_messages` and say to re-run `hyp init`.
- LLP 0213 corrects the two claims that made the check look redundant.

Docs and hygiene:

- Codex `activate()` JSDoc drops `hypaware-report`, which is not registered.
- Stale pointers to the retired skills: LLP 0067 gains the same Extended-by
  banner 0066 got; 0142's dead annotation bullets are struck; 0100 and 0196
  gain scoped supersession notes.
- LLP 0069's settled table cell is restored with an appended note, matching
  the append-not-replace pattern this PR set at LLP 0196.
- LLP 0197 keeps `#t12-graph-was-already-owned` on the finding a live @ref
  cites, with the supersession note above it unanchored.
- Both client manifests revert to their compact formatting, leaving six
  semantic lines instead of ~80.
- LLP 0216 carries the fixture `pattern` sources into D3, re-homes the
  never-`rm`-the-sources rule (which guards a still-local surface), and
  corrects "21 to 10" to 17 to 10.

Tests: rider opt-out survives a reconfigure, a picked plugin still loses a
stale flag, an excluded plugin declaring `compose_with` is not composed, no
excluded manifest declares it, the preset composes the pair in dependency
order, self-reference rejected, mutual pair allowed, and the typo'd-name
silent no-op is pinned.
@philcunliffe

Copy link
Copy Markdown
Contributor

Review round 1 of cf3baed (adopted PR, LLP 0024/0058). Verdict: findings - 12 total, 6 major. Ten are fixed and pushed as 3ae9e2e; two are deliberately left (one needs your call, one is out of scope).

The reviewer read the full diff against 207aaf1, ran the suite after a fresh npm install (3948 pass / 0 fail / 1 skipped - the PR body's claim of two pre-existing failures from stale node_modules does not reproduce; the suite is green), typecheck clean, llp-ref-hygiene 11/11, and all 11 new @ref anchors resolve.

The two consent and privacy findings, both fixed

2. major - compose_with silently erased a user's explicit enabled: false opt-out.
walkthrough.js:1243 did if (merged.enabled === false && composed.enabled === undefined) delete merged.enabled, justified by "Composing a plugin is what picking its row means". Sound for a picked plugin, but a rider is by construction not picked. The graph has no picker row (LLP 0213 #derived-data-plugins makes that a design commitment), so {"name": "@hypaware/context-graph", "enabled": false} is the only opt-out a user has, and any later hyp init deleted it without a word. dispatch.js:365 already treats enabled as a first-class user control, so this was a live consent regression, not hypothetical.

Fixed by scoping the delete: mergePlugin takes an isRider flag and returns before the delete. The fixer chose this over skipping riders in ridersFor for a reason worth recording: riders are in composerManagedPlugins, so a skipped rider's {enabled: false} entry would itself be dropped from the carried-forward config and the next hyp init would re-compose it clean. Scoping the delete keeps the flag permanently. Two tests: a rider's enabled: false survives a reconfigure, and the inverse (a picked plugin still loses a stale enabled: false) so the exception stays scoped.

3. major - compose_with could bypass V1_EXCLUDED_FROM_DEFAULT.
walkthrough.js:2011 built the catalog from [...bundled.loaded, ...bundled.excluded] and plugin_catalog.js:61-62 recorded compose_with for every manifest in that list without checking the allowlist. That allowlist is the explicit-opt-in boundary; its own comment names @hypaware/embedder-openai as excluded because enabling an API-backed embedder is the opt-in that lets captured text leave the machine. Nothing exploited it today, but a one-line manifest edit on any excluded plugin would have composed it into every gateway config with no pick and no prompt.

Fixed with a new exported ridersInDefaultSet(composeWith, loaded) filtering riders to the default-activated set, applied in loadPickerCatalog rather than buildPluginCatalog so the catalog stays a faithful manifest read. Tests stage the exact one-line manifest edit and assert it would ride unfiltered but does not after filtering, plus a forward guard that no excluded bundled manifest declares compose_with.

The other four majors, all fixed

1. hypaware-query deleted its graph-availability gate, but the graph does not reach every install. Both SKILL.md copies asserted "on a standard install: ai_gateway_messages, node, edge" and instructed running hyp graph unconditionally, on the strength of llp/0213:220-222 ("lists node and edge on every install"). That is false. compose_with is consumed only at walkthrough.js:1010; src/core/runtime/ is untouched, so bootProfile: 'config' is a literal read of config.plugins - and llp/0213:276-281 itself admits existing configs are not migrated. Not only legacy either: hyp init claude-and-otel-local writes a brand-new config with the gateway and no graph pair, so "New configs get the graph" was wrong on a shipped fresh-install path. hyp skills install pushes the rewritten skill to exactly those installs, where the model is told to use commands and datasets that do not exist, with the diagnostic removed. Fixed all three ways: diagnostic sentence restored to both copies, both graph plugins added to the preset, and llp/0213:216-233 rewritten so the doc matches the code (new #availability-is-not-universal).

4. Dangling reference in shipped runtime source. codex/src/index.js:49-50 still said activate() contributes hypaware-report after it was deleted from the register loop five lines below; the guard test only reads manifest.description so nothing caught it. Fixed.

5. Retired skills are never pruned from installed clients. runSkillsInstall only materializes currently-registered assets; removeClientAssets runs on detach, never on install. So hypaware-ignore/, hypaware-unignore/, hypaware-report/, hypaware-graph/ all remain on disk after upgrade, still model-invocable - and the copy left behind is the drifted one with the stale 8787 fallback that motivated LLP 0212. It does fail closed (set -euo pipefail plus curl --fail-with-body against a dead port exits nonzero). llp/0215:194-197 acknowledges this for hypaware-graph only, calling it "#660, which this makes one case worse and does not fix"; it is now five cases. NOT fixed - it is a pre-existing gap and teaching hyp skills install to prune is a feature change, out of scope here. Worth widening that LLP note and the release notes to name all five.

6. Stale pointers to deleted skills in Accepted/Active LLPs. LLP 0213 states the rule ("A retirement is finished when nothing points at it") and missed five sites: llp/0067:16-18 asserts in present tense that a now-deleted SKILL.md "is the contract" (0066 got its Extended-by banner, its design doc did not); llp/0142:118-121 still instructs annotating skills with zero surviving @ref sites; llp/0100:74-76 normative R-text says to opt out via /hypaware-ignore; llp/0196:185-186 rows still say "(unchanged)". All five fixed as mechanical stale-pointer repairs, which CLAUDE.md permits on Accepted docs.

Minors, fixed

  • 8. llp/0069:71 rewrote a settled table cell in place, inconsistent with how 0066/0142 were handled in this same PR. Fixed by restoring the original cell and appending a scoped note - the fixer noted this PR set that precedent itself at llp/0196:182, where the hypaware-report row keeps its full text and appends the supersession, so append-not-replace is demonstrably the house pattern and the in-place swap was the outlier. llp/0100 was given the identical shape.
  • 9. llp/0197:278 retargeted an anchor a live @ref depends on. client-skill-manifest-agreement.test.js:7 cites #t12-graph-was-already-owned, and that file is not in this PR's changed set, so the ref resolved to a retraction of the thing it cites. Ref-hygiene cannot catch it because the anchor still exists. Fixed by leaving the anchor on the original finding.
  • 10. compose_with failure modes untested and silent. Fixed: manifest validation now rejects a self-reference; tests added for that, for a mutual pair validating alone (documenting that one manifest's validation cannot see whether the pair stalls), and pinning the current silent-no-op on a typo'd package name. No warning system was invented.
  • 11. Both client manifests reformatted wholesale, inflating a three-line semantic change into ~80 diff lines, and diverging from gascity/hypaware.plugin.json's compact style. Fixed - reset to base and semantic changes re-applied by hand; now 7 and 5 lines.
  • 12. LLP 0216 handoff gaps. The "eleven constraints" claim checks out and matches the ids removed from the fixture. But the pattern field was not carried over, so the actionable form of no-wide-column-scans survived only in git history; and rendering.md:209 ("the source .md files are the record: never rm them") guards a still-local surface, since D2 keeps hyp report render, yet was recorded nowhere. Fixed: patterns carried into D3, a #recovered-rule-text section added for both, and "21 to 10" corrected to "17 to 10".

7. minor - LLP number collision. YOUR CALL, deliberately not touched

Open PR #716 adds llp/0212-status-is-a-triage-summary.decision.md; this PR adds llp/0212-session-opt-out-is-a-cli-verb.decision.md. Both are branched off 207aaf1. Different filenames, so git will merge both silently, and llp-ref-hygiene.test.js:458 ("no LLP number is claimed by two documents") only fires after the second merge. 0213 to 0216 are clear across all 16 open PRs.

Whichever merges second has to renumber, which is mechanical and permitted under LLP 0156. Neutral is not picking which, since that is cross-PR coordination affecting a PR it does not own this decision for. Worth deciding before you merge either, not after.

Also checked, clean

  • registerGroup / getGroup / verb help. The "metadata only, cannot shadow a command, cannot appear as its own subcommand" claim was verified by reading registry/commands.js, group_help.js, and dispatch.js, not by trusting the body: groups live in their own Map and list()/get()/has()/size()/match() all read byName, so it is exact. --help ordering is correct - dispatch.js:468-475 renders group help only when listGroupChildren is non-empty, so graph neighbors --help correctly hits renderCommandHelp with the verb's help. A plugin registering a group named query or daemon cannot override the core header, because a group with a bare command resolves through matched.command and never consults getGroup.
  • ridersFor termination. Fixpoint over a monotonically growing set with a grew flag; self-reference and mutual reference both terminate on the first pass without composing. No hang, no recursion. composerManagedPlugins correctly drops riders when a reconfigure unpicks the gateway, matching LLP 0183.
  • graphEmpty and local-only. NODE_COLUMNS/EDGE_COLUMNS carry no cwd, so query/visibility.js:157 can never withhold graph rows (only suppress natural_key/label). nodeById.size === 0 therefore genuinely means unprojected, and a restricted caller is never told to project a graph it simply cannot see.
  • Capability parity on the session opt-out. Premises verified: DEFAULT_GATEWAY_ENDPOINT is http://127.0.0.1:18521, so the deleted skills' 8787 fallback was genuinely stale; and LLP 0066 R9 requires the caveat to name both the restart and the fork-minted id, which the deleted skill named only the restart of. The replacement in both hypaware-reference copies names both plus the fail-closed behaviour, and docs/PRIVACY.md:107-115 and README.md:352-354 match.
  • Deletion sweep. Exhaustive grep for all four skill names and all seven stage filenames across .claude/, docs/, README.md, bin/, scripts/, all 40 smoke flows, package.json, the kernel types, and every test. Beyond findings 4 and 6, everything else is historical or a false positive on hyp report, which survives. hypaware-ignore/hypaware-unignore never existed in the codex tree, so the PR closed a pre-existing asymmetry rather than creating one. Manifest contributes.skills, ctx.skills.register, on-disk directories, and manifest description agree in all four directions across claude, codex, gascity, and context-graph.
  • LLP hygiene. All five new docs have well-formed headers with matching filename suffixes. The additive forward-ref banners on 0066, 0142, 0196, 0197, 0005, 0011 and 0000 follow each doc's own precedent and preserve the original text.
  • Conventions. No U+2014 anywhere in the changed files, no semicolons, no @typedef, no inline import types, type-import specifiers root-anchored.

Cosmetic, not fixed: llp/0215:46 cites walkthrough.js:1060 for composerManagedPlugins, now at 1109; llp/0215 is Status: Active while declaring "All eight landed"; llp/0196:177 says "10 to 6" where it is now 3.

Gates after the fixes: npm test 3958 pass / 0 fail / 1 skipped, typecheck exit 0, llp-ref-hygiene 11/11.

One note from the fixer worth passing on: typecheck initially failed on a new test file, so npm test alone does not catch type errors in tests here - the typecheck step is load-bearing.

The head has moved to 3ae9e2e, so the next tick reviews that head (round 2).

The round-1 guard against `compose_with` bypassing
`V1_EXCLUDED_FROM_DEFAULT` landed inside `loadPickerCatalog`, which
`resolvePickSeeding` only reaches when no catalog is injected. But
`runInitWizard` (the shipped `hyp init` entry point) always injects one,
built by `loadWizardCatalog` from the loaded *and* excluded manifests, so
the boundary held on the legacy walkthrough and not on the path that ships.

`ridersInDefaultSet` now filters against `V1_EXCLUDED_FROM_DEFAULT` itself
rather than a `loaded` array the caller has to supply: only one of the three
catalog sources has such an array, and a filter that depends on what the
caller passes is a filter a caller can skip. Both branches in
`resolvePickSeeding` run it, so no catalog source (wizard, injected, or
discovered) routes around it.

The new regression test goes through `resolvePickSeeding` with an injected
catalog rather than calling the filter directly: a unit test of the filter
cannot catch a caller that does not call it, which was the whole bug. It
fails if the `pick.js` filter is reverted.

Also restores the two annotation bullets LLP 0142 (Accepted) settled,
verbatim as at 207aaf1, with an appended `Superseded-by:` forward-ref on
each noting the annotation sites no longer exist. Matches how 0069, 0100,
and 0196 were handled in this PR: append, never replace.
@philcunliffe

Copy link
Copy Markdown
Contributor

Review round 2 of 3ae9e2e (adopted PR). Verdict: findings - 1 major, 1 minor, both fixed and pushed as cbda613. Nine of the ten round-1 fixes landed correctly; the tenth landed on the wrong code path.

1. major - the allowlist filter guarded the legacy path, not the shipped one. FIXED

Round 1's fix B (stop compose_with bypassing V1_EXCLUDED_FROM_DEFAULT) was written correctly and placed in loadPickerCatalog. But loadPickerCatalog is only reached when opts.catalog is absent (wizard/pick.js:76-78), and runInitWizard always supplies one (wizard/index.js:80), built by loadWizardCatalog from the raw buildPluginCatalog([...bundled.loaded, ...bundled.excluded]). Both hyp init call sites pass no catalog, so loadWizardCatalog is the live path - and walkthrough.js:594-597 says in its own words that runPickerWalkthrough is "superseded as hyp init's entry point by runInitWizard". The fix landed on the superseded surface.

The reviewer reproduced it against the committed tree: staging the one-line manifest edit the filter exists to defeat and calling resolvePickSeeding({ catalog, picks: ['claude'] }) yielded composeWith.has('@hypaware/central') === true, feeding straight into composePickerConfig.

Not currently exploitable (no shipped excluded manifest declares compose_with, and a test guards that), so it was a latent boundary rather than a live leak. But it was worse than the unfixed state in one respect: the round-1 test called ridersInDefaultSet directly and read as proof the boundary held, while production never called it.

Fixed by making the filter unavoidable rather than source-dependent. ridersInDefaultSet now reads V1_EXCLUDED_FROM_DEFAULT itself instead of an injected loaded array, and it is applied on the injected branch in pick.js too, so no catalog source (wizard, injected, or discovered) can route around it. Verified the discrimination: reverting only the pick.js line makes the new test fail (@hypaware/central is excluded from default, so it may not ride) while the round-1 test still passes - exactly the gap. No import cycle appeared; walkthrough.js already imported from runtime/bundled.js.

2. minor - llp/0142:116-128 deleted settled content and paraphrased it. FIXED

0142 is Accepted. Round 1's fix E removed two annotation bullets and replaced them with a paragraph describing the bullets that used to be there. That is still an edit to what the doc settled, and it was the outlier of the four: llp/0069:71 (fix F), llp/0100:79-82 and llp/0196:185-186 all restore-and-append. Fixed - both bullets restored byte-verbatim from 207aaf1 with Superseded-by: forward-refs appended, keeping the closing sentence as the trailing note.

Verified from round 1

  • A (consent) - landed, and holds under attack. isRider is computed from composeWith?.keys(), the same set ridersFor draws from and composerManagedPlugins adds - one source, three consumers, no drift. mergePlugin has a single call site, so the exception cannot leak into another merge path, and the early return sits after the config/upstreams merge so a rider still gets its config folded. Riders are pushed as bare { name }, so the spread preserves enabled: false rather than overwriting it with undefined; config/apply_deps.js:110,196 both honor it downstream. One noted non-finding: a plugin that were both picked and a rider would be treated as a rider and keep a stale enabled: false. Not reachable today (no picker descriptor's compose block names either graph plugin), and the failure direction is the safe one, but worth a sentence in the mergePlugin doc if a descriptor ever composes a rider.
  • B (privacy boundary) - the function was right, its coverage was not. See finding 1. Every property round 2 was asked to attack held: it filters the rider (the correct side of the relation, not the plugins it waits for); a second-order chain cannot smuggle an excluded plugin in, because a chain only extends through rider keys and every key is filtered; and buildPluginCatalog's six other callers never read composeWith, so leaving the catalog unfiltered is safe there. The one unfiltered reader was wizard/pick.js, and it was the live one.
  • C (graph availability) - landed, all three parts. The diagnostic sentence is byte-identical in both SKILL.md copies; claude/src/index.js:382-389 names both graph plugins in the preset, engine before connector; llp/0213:220-237 carries the new #availability-is-not-universal. The other bundled preset (gascity/src/init.js) has no gateway, so nothing else needed widening.
  • D through J - all landed. D: the codex JSDoc no longer names hypaware-report. E: four of five (the fifth was finding 2). F: llp/0069:71 restored with the note; 0100 matches. G: the anchor is back on the finding at llp/0197:285, with an explicit sentence saying why it must not move onto its own retraction. H: manifest.js:159-161 rejects a self-reference, correctly ordered after the name and isStringArray guards. I: reverted cleanly - the manifest diff against base is now four hunks (two description sentences, two skills arrays), every retained key byte-identical to base, nothing semantic lost. J: llp/0216:104-128 carries a pattern for each of the eleven, #recovered-rule-text is present, and "17 to 10" is arithmetically true against the merge base.

Still open from round 1, carried forward

Also checked, clean

  • Rider fold semantics. ridersFor terminates: present only grows and grew is set only on a new name, so the fixpoint is bounded by map size. A mutual pair stalls silently rather than looping, which llp/0005:96-104 now documents.
  • composerManagedPlugins still includes riders, so the stranding rule survives fix A - the consent exception changes only what enabled: false means, not whether an unpicked gateway drops the pair.
  • No dangling references to hypaware-report, hypaware-ignore, hypaware-unignore or hypaware-graph survive in any manifest, src/, plugin source, README.md or docs/. The two live mentions are deliberate @ref comments.
  • LLP 0214 surface. registerGroup is metadata-only and never enters list(), so it cannot shadow a command. The if (groupCommand) to if (groupCommand?.summary) change is a strict improvement: it stops a help-only group printing hyp graph - undefined.
  • graphEmpty plumbing is set on the shared operation result rather than in CLI rendering, so the MCP caller and the terminal get the same distinction.
  • docs/PRIVACY.md and README.md re-point the session opt-out and graph prose correctly, and the PRIVACY rewrite adds the fork-and-restart caveat rather than just swapping the surface name.
  • Conventions across the full diff: zero U+2014, no @typedef, no inline import types, root-anchored type-import specifiers, no stray semicolons.

Gates after the fixes: npm test 3959 pass / 0 fail / 1 pre-existing skip, typecheck exit 0, llp-ref-hygiene 11/11.

The head has moved to cbda613, so the round budget (2) is spent at an unreviewed head: the next tick triages rather than opening a round 3.

@philcunliffe

Copy link
Copy Markdown
Contributor

Triage after the review budget (LLP 0017). Two review rounds ran (14 findings, 12 fixed). Both residuals were judged non-blocking, so this PR can ship.

(A) Retired skills are never pruned on upgrade - deferred to #726. The mechanism was confirmed by reading: materializeClientAssets only copies currently-registered assets and has no removal step, and removeClientAssets runs only on detach. So hypaware-ignore, hypaware-unignore, hypaware-report (both clients) and hypaware-graph survive an in-place upgrade, still model-invocable. Two facts kept it out of blocker territory: the retired skill fails closed (set -euo pipefail plus curl --fail-with-body against a dead port exits nonzero and the text says the session is "still being recorded", so it never falsely reports an opt-out), and the 8787 fallback is not novel to it - the currently shipped hypaware-privacy/SKILL.md carries the same hardcoded fallback in both client copies today, which LLP 0212 explicitly scopes out under LLP 0066 R14. A user who never re-runs hyp skills install keeps exactly the file they already had pre-PR, so merging this does not regress anyone; it adds four cases to known issue #660.

(B) The three-way LLP 0212 collision - repo-hygiene and merge sequencing, not a software defect. LLP docs are not runtime code and nothing acts on a doc's number. See the merge-time note below, which matters more than the classification.

Merge-time note, please read before merging any of the three

llp/0212-session-opt-out-is-a-cli-verb.decision.md here collides with a different llp/0212-*.decision.md in #716 (0212-status-is-a-triage-summary) and #721 (0212-one-pushdown-converter). The filenames differ, so git merges all three without conflict, and test/core/llp-ref-hygiene.test.js only catches the double-claim once the second one lands - failing on whichever PR merges after the first, not on the one that caused it.

The repo already has a settled procedure: llp/0156 says the number goes to whichever document reaches master first, and the later claimant is renumbered in its own landing commit (already applied to 0098, 0099, 0111, and PR #486's 0142). LLP 0156 also classifies that renumbering as a permitted mechanical edit even on an Accepted doc.

If #720 is not merged first, its sweep touches 13 files: the doc, 2 @ref sites (hypaware-reference/SKILL.md:87 in both client copies), 1 test comment (test/plugins/ai-gateway-session-ignore-receipt.test.js:150), and 12 cross-references across 9 other LLP docs (0100, 0069, 0216 twice, 0197, 0066, 0215, 0196 twice, 0142 three times, 0067). For #721 the sweep is 3 files. Worth deciding the landing order deliberately rather than letting it fall out.

@philcunliffe philcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Aug 12, 2026
@bgmcmullen
bgmcmullen merged commit 5b13456 into master Aug 12, 2026
9 checks passed
@bgmcmullen
bgmcmullen deleted the skills-surface-reduction branch August 12, 2026 23:19
philcunliffe added a commit that referenced this pull request Aug 14, 2026
…#750) (#757)

* walkthrough_picker_to_first_query golden predates compose_with riders (#750)

The Phase 5 golden config still lists the six plugins the picker rows
compose, but `hyp init` has written eight since #720 landed LLP 0213 D1:
`@hypaware/context-graph` and `@hypaware/ai-gateway-graph` declare
`compose_with: ["@hypaware/ai-gateway"]` and ride the gateway pick.

The picker is right and the golden is stale. #720 added the rider fold to
`composePickerConfig` (+153 lines in `src/core/cli/walkthrough.js`) and
touched no smoke flow, which is why neither `goldenPickerConfig` nor
`bundled.js` shows the change in `git log`: the divergence arrived through
the manifests.

Rather than paste the two names in, the golden derives its rider tail from
the same declarations the composer reads, so the next derived-data plugin
is a manifest edit rather than a re-red smoke. The literal half is what the
picker actually decides: which rows compose, the upstreams, the OTLP port,
the sink shape, retention. The derivation reads manifests, not
`ridersFor`, so a plugin that is neither picked nor declared a rider still
fails the assertion; the rider mechanism itself keeps its unit coverage in
`test/core/compose-picker-config.test.js`.

Co-Authored-By: Claude <noreply@anthropic.com>

* Make composedRiders a fixpoint and fix its default-activation comment (#750)

composedRiders was single-pass while ridersFor runs to a fixpoint, so a
rider whose compose_with names another rider (rather than a picked
plugin) was composed by the implementation but missing from the
golden, false-redding the smoke on a manifest-only edit. Run the same
manifest set to a fixpoint, mirroring ridersFor, so second-order
riders land in the golden too.

Also corrects a comment that claimed discoverBundledPlugins's loaded
bucket is "the same cut" ridersInDefaultSet makes on the composer.
It is not the same cut (allowlist membership vs. exclude-set
non-membership); they only agree because those two lists partition
the whole bundled workspace and loadPickerCatalog reads only them.

---------

Co-authored-by: test <test@test.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:adopt Foreign PR adopted into neutral's reconcile scope neutral:adopted Adoption completion record: merged while carrying neutral:adopt (LLP 0031) neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants