Skip to content

HypAware plugin surface extensibility#35

Merged
philcunliffe merged 7 commits into
masterfrom
integration/hypaware-plugin-surfaces
May 26, 2026
Merged

HypAware plugin surface extensibility#35
philcunliffe merged 7 commits into
masterfrom
integration/hypaware-plugin-surfaces

Conversation

@philcunliffe
Copy link
Copy Markdown
Contributor

@philcunliffe philcunliffe commented May 26, 2026

What It Does

This PR is the plugin-surface integration branch. It brings together the stacked work for config-driven sinks, manifest-owned product behavior, the TUI init flow, and Gas City acceptance cleanup.

  • Boots the CLI and daemon through the plugin catalog so bundled and installed manifests contribute capabilities, commands, init presets, datasets, skills, and client descriptors.
  • Materializes configured sinks from config at boot for request sinks, encoder/blob-store sinks, and table-format sinks.
  • Moves Claude/Codex-specific attach and status behavior into manifest contributes.client descriptors, including attach probes and required gateway upstreams.
  • Adds plugin-owned skill install behavior and Claude session-context capture used by the projector.
  • Adds the TUI prompt runtime/tests and updates the init walkthrough.
  • Includes review fixes: first-party client diagnostic fallback when catalog discovery is absent or partial, descriptor-derived gateway_missing_*_upstream diagnostic kinds, direct attach-probe path-helper coverage, and visible CLI warnings when boot-path sink materialization fails.

What Breaks / Compatibility Notes

Validation

  • npm test (441 tests)
  • npm run typecheck
  • npm run lint

philcunliffe and others added 2 commits May 25, 2026 20:32
* feat: config-backed sink materialization (hy-9d39h)

Add production runtime materialization for configured sinks, so
daemon boot and CLI dispatch automatically instantiate sinks from
config.sinks entries without manual kernel.sinks.instantiate() calls.

- Add `fromProvider(provider, name, range)` to capability registry for
  provider-specific capability lookup
- New `src/core/sinks/materialize.js` resolves writer/destination/encoder
  capabilities and calls sinks.instantiate() for all three sink shapes
  (request, blob, table-format)
- Wire materializeSinks into daemon runtime (after plugin activation)
  and CLI dispatch (after boot)
- Update local_parquet_export smoke to use config-backed sinks instead
  of manual instantiation, proving the production path works
- 19 new tests covering all sink shapes, error modes, and fromProvider

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(types): resolve CI typecheck failures for sink materialization (hy-9d39h)

- Add tmpRoot to RunDaemonOptions interface
- Add fromProvider to CapabilityRegistry interface and activation facade
- Capture config.sinks before withSpan callback to preserve TS narrowing

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
philcunliffe and others added 2 commits May 25, 2026 21:10
…ills, hook migration (hy-bv310) (#38)

Move client-specific and plugin-specific product behavior out of core
and behind plugin-owned manifest descriptors:

- Add `contributes.client` descriptor to plugin manifests with
  skill_dir, attach_probe, and required_upstreams fields
- Extend plugin catalog to extract client descriptors from manifests
- Replace hardcoded client loops in status.js with catalog-driven
  descriptor iteration for attach probing
- Generalize `hyp skills install` to resolve skill directories from
  catalog instead of hardcoding .claude/skills and .codex/skills
- Move `claude-hook session-context` command from core to the Claude
  plugin — registered during plugin activation
- Rewrite V1 advisory diagnostics to derive client/upstream checks
  and encoder/blob-store checks from catalog metadata instead of
  hardcoded plugin name sets

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reframe gascity from "excluded and invisible" to "excluded from default
activation but discoverable through the plugin catalog." Update test
and smoke labels/comments to reflect that gascity absence in status is
about the config scenario, not a blanket prohibition. Add catalog test
for gascity contributions (source, commands, dataset, init preset,
skill). Add detach coverage to the gascity smoke test.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@philcunliffe
Copy link
Copy Markdown
Contributor Author

Dual-agent review — request_changes

  • Verdict: request_changes
  • Risk class: medium
  • Auto-merge advisory: 👎 thumbs down - verdict is request_changes; changes still need human-gated follow-up
  • Blast-radius bead: hy-7hcu1

Auto-merge advisory

👎 thumbs down - verdict is request_changes; changes still need human-gated follow-up

Advisory only: no merge was attempted.

Risk capstone

Cross-reference: reviewer findings that intersect high-risk surfaces

Source Finding (severity, evidence) Intersects
Claude review resolveClientSettingsPath env var derivation bug (major, status.js:587) Risks: "attach_probe sub-fields lack dedicated unit tests — resolveClientSettingsPath() exercised via smoke tests only"
Claude review Inline import('...') types across 4 files (CLAUDE.md violation, multiple files) (none — style/contract issue, no blast-radius surface)

Blast radius

  • Boot-path sink materialization errors are logged but not surfaced to the user in CLI dispatch pathdispatch.js:119 calls materializeSinks() but does not check or display sinkResult.errors. Daemon path (runtime.js:222) logs errors to fileLog. CLI callers get silent failures.
  • attach_probe sub-fields lack dedicated unit testsprobeClientAttachFromDescriptor() and resolveClientSettingsPath() are exercised via smoke tests (hyp status output) but have no isolated unit tests. A subtle regression (wrong env override, path join error) would only surface in integration.
  • Stacked integration branch with 4 commits — merge into master carries 3 feature PRs (feat: config-backed sink materialization (hy-9d39h) #37, feat: plugin-owned product logic (hy-bv310) #38, feat: gascity acceptance and cleanup (hy-sadjq) #39) atomically. If any single feature has a regression, bisecting requires looking inside the integration branch. Consider whether the integration branch should be merged as a merge commit (preserving individual commits) or squashed.
Claude review

Code review

Found 2 issues:

  1. Inline import('...') types used instead of @import declarations across 4 files (CLAUDE.md says "Never use inline import('...') types. Declare type imports at the top of the file with @import JSDoc comments, then reference the bare names.")

    This PR introduces 20+ new inline import('...') type annotations. Each should be replaced with an @import declaration at the top of the file and a bare type name reference. Affected locations:

    • validate.js:620 -- import('../plugin_catalog.js').ClientDescriptor in @param
    • status.js:121 -- import('../plugin_catalog.js').PluginCatalog in @type
    • status.js:124,126 -- import('../manifest.js').LoadedManifest in @type
    • status.js:530 -- import('../plugin_catalog.js').ClientDescriptor in @param
    • materialize.js:265-266 -- import(...).PluginActivationContext in @param
    • sink-materialize.test.js -- 14 instances of import(...).HypAwareV2Config in type casts

    * @param {HypAwareV2Config | null | undefined} config
    * @param {{ clientDescriptors?: Map<string, import('../plugin_catalog.js').ClientDescriptor>, knownPlugins?: Map<PluginName, PluginMetadata> }} [ctx]
    * @returns {V1Diagnostic[]}

    /** @type {import('../plugin_catalog.js').PluginCatalog | undefined} */
    let catalog
    try {
    /** @type {import('../manifest.js').LoadedManifest[]} */
    let bundledLoaded = []
    /** @type {import('../manifest.js').LoadedManifest[]} */
    let installedLoaded = []

    *
    * @param {{ descriptor: import('../plugin_catalog.js').ClientDescriptor, homeDir: string, env?: NodeJS.ProcessEnv }} args
    * @returns {Promise<{ attached: boolean, settingsPath?: string, version?: string, port?: string, error?: string }>}

    * @param {PluginName} destName
    * @param {import('../../../collectivus-plugin-kernel-types.d.ts').PluginActivationContext} writerCtx
    * @param {import('../../../collectivus-plugin-kernel-types.d.ts').PluginActivationContext} destCtx
    * @param {TableFormatProvider} tableFormat

    const config = /** @type {import('../../collectivus-plugin-kernel-types.d.ts').HypAwareV2Config} */ ({
    version: 2,

  2. resolveClientSettingsPath produces invalid env var names for hyphenated client names (bug due to clientName.toUpperCase() without sanitizing non-alphanumeric characters)

    The function derives an env-override key as `${clientName.toUpperCase()}_HOME`. For clientName = 'codex' this gives CODEX_HOME (correct). But for any client name containing a hyphen (e.g. 'claude-desktop'), this produces CLAUDE-DESKTOP_HOME -- hyphens are not valid in POSIX environment variable names, so process.env[envKey] will always be undefined and the env override silently fails. Additionally, the function now checks CLAUDE_HOME for the Claude plugin, but the Claude plugin has never supported this env var, creating a risk of spurious path resolution if a user has CLAUDE_HOME set for unrelated reasons. Fix: sanitize with clientName.toUpperCase().replace(/-/g, '_').

    function resolveClientSettingsPath(clientName, settingsFile, env, homeDir) {
    const envKey = `${clientName.toUpperCase()}_HOME`
    const override = env?.[envKey]
    if (typeof override === 'string' && override.length > 0) {
    const parts = settingsFile.split('/')
    return path.join(override, ...parts.slice(1))
    }
    return path.join(homeDir, ...settingsFile.split('/'))
    }

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

  • Codex review: (no findings reported)

Reports: /Users/phil/testcity/.gc/pr-pipeline/reviews/pr-35 · Bead: hy-v124h · Blast-radius: hy-7hcu1

…ze env var derivation (hy-hfne7) (#40)

Replace 20+ inline import('...') type annotations with @import JSDoc
declarations at file tops across validate.js, status.js, materialize.js,
and sink-materialize.test.js.

Fix resolveClientSettingsPath to sanitize non-alphanumeric characters
(hyphens, dots, etc.) in client names when deriving env var keys, so
that hyphenated clients like 'claude-desktop' produce valid POSIX env
var names (CLAUDE_DESKTOP_HOME instead of CLAUDE-DESKTOP_HOME).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@philcunliffe
Copy link
Copy Markdown
Contributor Author

Dual-agent review — block

  • Verdict: block
  • Risk class: medium
  • Auto-merge advisory: 👎 thumbs down - verdict is block; changes still need human-gated follow-up
  • Blast-radius bead: hy-av2tv

Auto-merge advisory

👎 thumbs down - verdict is block; changes still need human-gated follow-up

Advisory only: no merge was attempted.

Risk capstone

Cross-reference: reviewer findings that intersect high-risk surfaces

Source Finding (severity, evidence) Intersects
Claude (major) V1DiagnosticKind type union stale after generalization to dynamic template strings (types.d.ts:62-67, validate.js:659) Risks: "diagnoseV1Config kind values are now dynamic"
Claude (major) diagnoseV1Config silently produces no diagnostics when plugin catalog discovery fails (validate.js:630-635) Targets: diagnoseV1Config (modified signature); Direct callers: clientDescriptors in validate.js:630,633
Claude (major) resolveClientSettingsPath and probeClientAttachFromDescriptor lack unit tests (status.js:535-538, status.js:588-596) Risks: "attach_probe sub-fields lack dedicated unit tests"

Blast radius

  • Boot-path sink materialization errors are logged but not surfaced to the user in CLI dispatch pathdispatch.js:119 calls materializeSinks() but does not check or display sinkResult.errors. Daemon path (runtime.js:222) logs errors to fileLog. CLI callers get silent failures.

  • attach_probe sub-fields lack dedicated unit testsprobeClientAttachFromDescriptor() and resolveClientSettingsPath() are exercised via smoke tests (hyp status output) but have no isolated unit tests. A subtle regression (wrong env override, path join error) would only surface in integration.

  • Stacked integration branch with 4 commits — merge into master carries 3 feature PRs (feat: config-backed sink materialization (hy-9d39h) #37, feat: plugin-owned product logic (hy-bv310) #38, feat: gascity acceptance and cleanup (hy-sadjq) #39) atomically. If any single feature has a regression, bisecting requires looking inside the integration branch. Consider whether the integration branch should be merged as a merge commit (preserving individual commits) or squashed.

  • diagnoseV1Config kind values are now dynamic — diagnostic kind field is now computed as `gateway_missing_${descriptor.requiredUpstreams[0]}_upstream` (validate.js:659). External consumers parsing kind strings (e.g. automation, dashboards) may need updating. Previously the kinds were fixed literals (gateway_missing_anthropic_upstream, gateway_missing_openai_upstream).

  • Codex review: (no findings reported)

Claude review

Code review

Found 3 issues:

  1. V1DiagnosticKind type union is stale after generalization to dynamic descriptor-driven diagnostics (bug due to type cast masking mismatch in validate.js)

    The closed string-literal union V1DiagnosticKind in types.d.ts enumerates only gateway_missing_anthropic_upstream and gateway_missing_openai_upstream. The new code at validate.js:659 generates kind strings dynamically via template literal: `gateway_missing_${descriptor.requiredUpstreams[0]}_upstream`, then suppresses the type error with /** @type {V1DiagnosticKind} */. Any future plugin whose requiredUpstreams[0] is neither anthropic nor openai will produce a kind string outside the union, undetected by the type system. The PR's goal is to generalize from hardcoded plugin names, but the type system doesn't follow the generalization.

    */
    export type V1DiagnosticKind =
    | 'client_without_gateway'
    | 'gateway_missing_anthropic_upstream'
    | 'gateway_missing_openai_upstream'
    | 'sink_missing_encoder'

    out.push({
    kind: /** @type {V1DiagnosticKind} */ (`gateway_missing_${descriptor.requiredUpstreams[0]}_upstream`),
    pointer: pluginPointer(config, AI_GATEWAY_PLUGIN),

  2. diagnoseV1Config silently produces no diagnostics when plugin catalog discovery fails (bug due to regression from hardcoded constant to fallible disk discovery)

    The old code used a hardcoded CLIENT_PLUGINS set that was immune to discovery failures. The new code builds the client list from ctx.clientDescriptors, which is populated by discoverBundledPlugins() + buildPluginCatalog() in collectHypAwareStatus. If either throws (disk permission error, corrupted manifest), catalog remains undefined, the fallback ctx.clientDescriptors ?? new Map() at validate.js:631 produces an empty map, and the diagnostic loop at lines 634-671 never executes. hyp status would show a clean output with no client_without_gateway or gateway_missing_*_upstream warnings, hiding real configuration problems.

    const gatewayConfig = enabledByName.get(AI_GATEWAY_PLUGIN)
    const clientDescriptors = ctx.clientDescriptors ?? new Map()
    const knownPlugins = ctx.knownPlugins ?? firstPartyPluginMetadata()
    for (const [clientName, descriptor] of clientDescriptors) {
    const pluginName = descriptor.plugin

  3. resolveClientSettingsPath and probeClientAttachFromDescriptor lack unit tests (CLAUDE.md says "Add traditional tests for deterministic logic: config parsing and validation, manifest validation, daemon install rendering, path helpers")

    These are path helpers with non-trivial logic: env-var override derivation, path segment replacement, and file existence probing. A prior env-var sanitization bug (hyphen in CLAUDE-DESKTOP_HOME) was fixed by PR fix: replace inline import types with @import declarations and sanitize env var derivation (hy-hfne7) #40, but the fix itself has no regression test. Subtle path-resolution regressions would only surface in integration.

    async function probeClientAttachFromDescriptor({ descriptor, homeDir, env }) {
    if (!homeDir || !descriptor.attachProbe) return { attached: false }
    const probe = descriptor.attachProbe
    const settingsPath = resolveClientSettingsPath(descriptor.name, probe.settings_file, env, homeDir)

    function resolveClientSettingsPath(clientName, settingsFile, env, homeDir) {
    const envKey = `${clientName.toUpperCase().replace(/[^A-Z0-9]/g, '_')}_HOME`
    const override = env?.[envKey]
    if (typeof override === 'string' && override.length > 0) {
    const parts = settingsFile.split('/')
    return path.join(override, ...parts.slice(1))
    }
    return path.join(homeDir, ...settingsFile.split('/'))
    }

Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.


Reports: /Users/phil/testcity/.gc/pr-pipeline/reviews/pr-35 · Bead: hy-u7r64 · Blast-radius: hy-av2tv

@philcunliffe philcunliffe marked this pull request as ready for review May 26, 2026 18:25
@philcunliffe philcunliffe merged commit 8c5dcdb into master May 26, 2026
6 checks passed
@philcunliffe philcunliffe deleted the integration/hypaware-plugin-surfaces branch May 26, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant