Update hypaware-query skill and add subagent

[bgmcmullen]

• New hypaware-analyst subagent: a fan-out worker for parallel analysis of local HypAware recordings (one agent per date/gateway/conversation slice), returning structured summaries instead of raw query output.
• hypaware-query SKILL.md (claude + codex) corrected to the real installed CLI: removed nonexistent catalog/logs/traces/metrics subcommands, added SQL dialect notes (JSON_EXTRACT pitfalls, regex extraction pattern), per-table schema guidance, and stderr guardrails.

[philcunliffe]

Dual-agent review — request_changes

• Verdict: request_changes
• Risk class: medium
• Auto-merge advisory: 👎 thumbs down — verdict is request_changes; needs human-gated follow-up

Advisory only: no merge was attempted.

Risk capstone

Cross-reference: reviewer findings vs high-risk surfaces

Source	Finding (severity, evidence)	Intersects
Codex	Security surface: path-traversal via agent.name (major) — activation.js:276, core_commands.js:3124, walkthrough.js:1172	Targets (createAgentRegistry, runAgentsInstall, walkthrough agents.install); Risks #1. Independently confirmed by Claude follow-up.
Claude	Walkthrough finale install untested (major) — walkthrough.js:1155-1192	Direct callers (walkthrough finale); Risks #2
Claude	CLI install error/early-return paths untested (minor) — core_commands.js:3101-3133	Targets (runAgentsInstall); Risks #3

Codex review

Fix Validations

hypaware-query skill referenced nonexistent query subcommands

• Status: correct
• Evidence: src/core/cli/core_commands.js:86, src/core/cli/core_commands.js:92, src/core/cli/core_commands.js:98, src/core/cli/core_commands.js:104, src/core/cli/core_commands.js:110, hypaware-core/plugins-workspace/claude/skills/hypaware-query/SKILL.md:34, hypaware-core/plugins-workspace/codex/skills/hypaware-query/SKILL.md:34
• Assessment: The skill docs now match the registered hyp query subcommands: schema, status, sql, refresh, and maintain. The prior catalog/logs/traces/metrics guidance is removed and replaced with SQL/status discovery guidance.

SQL/stderr guardrails were missing from the skill

• Status: correct
• Evidence: hypaware-core/plugins-workspace/claude/skills/hypaware-query/SKILL.md:38, hypaware-core/plugins-workspace/claude/skills/hypaware-query/SKILL.md:59, hypaware-core/plugins-workspace/codex/skills/hypaware-query/SKILL.md:38, hypaware-core/plugins-workspace/codex/skills/hypaware-query/SKILL.md:59
• Assessment: The updated skill documents the JSON_EXTRACT pitfall and explicitly tells agents not to discard stderr, which addresses the described operational failure mode.

Findings

6) Security Surface

• Severity: major
• Confidence: high
• Evidence: src/core/runtime/activation.js:275, src/core/runtime/activation.js:287, src/core/cli/core_commands.js:3124, src/core/cli/walkthrough.js:1172
• Why it matters: agent.name is only validated as a non-empty string, then interpolated into filesystem destinations, so a plugin can register a name containing .. or path separators and cause hyp agents install or onboarding to copy outside the intended <agent_dir>.
• Suggested fix: Reject agent names that are not a safe basename, for example disallow /, \, .., absolute paths, and empty/hidden traversal segments in agents.register; also resolve the final path and assert it remains under path.resolve(homeDir, agentDir). Add regression coverage for traversal names through both CLI install and walkthrough install.

No Finding

• 1. Behavioral Correctness
• 2. Contract & Interface Fidelity
• 3. Change Impact / Blast Radius
• 4. Concurrency, Ordering & State Safety
• 5. Error Handling & Resilience
• 7. Resource Lifecycle & Cleanup
• 8. Release Safety
• 9. Test Evidence Quality
• 10. Architectural Consistency
• 11. Debuggability & Operability

Evidence Bundle

• Changed hot paths: agent registry creation; ctx.agents activation/dispatch wiring; hyp agents install; picker finale agent install; client descriptor cataloging of agent_dir; Claude plugin agent registration; hypaware-query skill docs.
• Impacted callers: src/core/cli/dispatch.js:183; src/core/cli/core_commands.js:2430; src/core/cli/core_commands.js:2636; src/core/cli/core_commands.js:3101; src/core/cli/walkthrough.js:783; src/core/cli/walkthrough.js:1167; hypaware-core/plugins-workspace/claude/src/index.js:253.
• Impacted tests: test/core/agents-install.test.js:21; test/core/agents-install.test.js:56; test/core/agents-install.test.js:87; test/core/agents-install.test.js:116; test/core/config.test.js:291; test/core/sink-materialize.test.js:128.
• Unresolved uncertainty: I did not run npm test; this review used the diff plus targeted call-site tracing.

Claude review

Claude review

Five parallel reviewers (guidance compliance, shallow bug scan, historical
context, contracts & callers, comments & tests) examined PR #86 at its head
commit (f4a9c92) in a detached worktree. The scoped core suite passes
(516 pass / 0 fail / 1 skipped once node_modules is resolved). The
implementation is unusually clean: the new agents contribution type is a
faithful, fully-wired mirror of the existing skills machinery — registry,
CLI install, walkthrough finale, plugin_doctor, manifest descriptor, types,
docs anchor, and packaging are all consistent. No logic bugs, no contract
drift, no dropped guard in history, and no guidance violations (no
semicolons, no inline import() types, no new @typedef, .d.ts
interfaces used correctly).

The only findings surviving the ≥80 confidence filter are test-coverage
gaps. (Reviewers independently agreed the code is correct; these are
about unexercised new behavior, not defects.) The shallow-bug-scan and
contracts reviewers did not surface the path-traversal issue Codex found;
on follow-up that finding was independently confirmed (see risk capstone).

Walkthrough finale agent-install path has zero test coverage

• Severity: major
• Confidence: 80
• Evidence: src/core/cli/walkthrough.js:1155-1192 (new agents.install span block: dry-run rendering, real mkdir+copyFile, descriptorMap gating, summary.agentsInstalled); finale count line at src/core/cli/walkthrough.js:959-962. No test references agentsInstalled, agents.install, or Would install agent (grep across test/ matches only the unit file).
• Why it matters: The walkthrough is the primary path users actually hit during onboarding; its new agent-install branch ships completely unexercised, so a regression in the real install, dry-run, or gating logic is invisible to CI — directly against the repo's "add traditional tests for deterministic logic" and log-driven-verification guidance.
• Suggested fix: Add a runPickerFinale/walkthrough test (the existing walkthrough-*.test.js files provide a harness) that, with an agents.list() stub and a picked claude client, asserts: (a) real install writes <home>/.claude/agents/<name>.md; (b) dryRun: true emits (dry-run) Would install agent and writes nothing; (c) summary.agentsInstalled is populated.

hyp agents install error / early-return paths untested

• Severity: minor
• Confidence: 88
• Evidence: src/core/cli/core_commands.js:3101-3111 ((no agents registered) early return; HOME is not set → exit 1) and src/core/cli/core_commands.js:3130-3133 (per-agent copyFile failure → warning: … failed:). None of these branches/strings are asserted in test/core/agents-install.test.js.
• Why it matters: The empty-registry, missing-HOME (distinct exit code 1), and copy-failure branches are real operator-facing behaviors; a refactor could silently change an exit code or swallow the warning with no test to catch it.
• Suggested fix: Add three small cases: empty registry → stdout (no agents registered), exit 0; env without HOME → exit 1 + HOME is not set; an agent whose sourceFile is missing → exit 0, stderr matches failed:, count line shows 0.

---

Reports: /Users/phil/workspace/hypaware/.git/dual-review/pr-86

[philcunliffe]

🧭 Decision map — where to spend your attention

This casts no verdict — it points at the 4 forks where the author made a real choice, so you can skim the rest.

Scanned: 59 hunks across 23 files (+711 / −45). Most is mechanical: a new agents registry threaded through every site that already enumerates skills (dispatch, activation, plugin_doctor, runInit/runPickerInit, type decls, test stubs), the hypaware-query SKILL.md doc correction duplicated across the claude + codex copies, and three new test files. The decisions worth your eyes, in order:

1. Path-traversal containment for plugin-supplied install paths · contract / security

src/core/runtime/contribution_names.js:25 · install recheck at core_commands.js:3131

export function isSafeContributionName(name) {
  if (name === '.' || name === '..') return false
  if (name.includes('/') || name.includes('\\')) return false
  if (path.isAbsolute(name)) return false
  return path.basename(name) === name

• Decision: Contribution name crosses the core/plugin trust boundary and is interpolated into <dir>/<name>.md, so core validates it at registration (this also retroactively closes the same gap for skills.register, activation.js:247) and re-checks containment at install via isWithinDir.
• Alternative not taken: Trust plugin-supplied names — which is exactly what skills.register did before this PR.
• Check: isWithinDir(dest, baseDir) compares dest against baseDir = path.join(homeDir, agentDir), but agentDir itself comes from the untrusted manifest. With dest = baseDir/<safe-name>.md the test passes for any base — so it guards a bad name, not a bad dir. Worth confirming a manifest agent_dir: "../../tmp/evil" is actually contained.

2. Fan-out worker pinned to model: haiku · magic value

hypaware-core/plugins-workspace/claude/agents/hypaware-analyst.md:5

model: haiku

• Decision: Every analyst worker runs on Haiku.
• Alternative not taken: Inherit the lead's model, or pin Sonnet — the worker's whole job is writing correct DuckDB SQL (JSON_VALUE, regexp_extract, dedup-before-sum) against unfamiliar schemas.
• Check: Haiku reliably produces that SQL; the ×N-fan-out cost win is worth it vs. a worker silently returning wrong aggregates.

3. Agents are a flat single file, not a skill-style directory · contract / shape

collectivus-plugin-kernel-types.d.ts:1317 (sourceFile, vs skills' sourceDir; installed via copyFile)

 * skills (a directory tree around a `SKILL.md`), an agent is a single
 * markdown definition file installed flat into the per-client agent
 * directory as `<agent_dir>/<name>.md`.

• Decision: An agent contribution is one .md file copyFile'd flat — deliberately not the skill model (sourceDir + recursive copy).
• Alternative not taken: Model agents as directories like skills, so an agent could later ship sibling resources. Expensive to widen once plugins author against the single-file shape.
• Check: Agents will never need to bundle adjacent files, and one flat .md matches each client's real on-disk agent convention.

4. Missing agent_dir → skip-and-warn (exit 0); Codex gets none · unhappy-path policy / scope

src/core/cli/core_commands.js:3124 + config/validate.js (Claude declares .claude/agents, Codex omits it)

ctx.stderr.write(`warning: agent '${agent.name}' targets client '${targetClient}' without an agent directory\n`)
continue

• Decision: A targeted client with no agent_dir is skipped with a warning and the command still exits 0; Codex declares none, so agents are Claude-only for now.
• Alternative not taken: Hard-error on a misconfigured target, or give Codex an agent concept now.
• Check: Silent no-op (not failure) is the intended behavior when an agent targets a client that can't receive it.

---

Honorable mentions (real but lower-stakes): hypaware-analyst.md:4 — the "read-only" analyst is granted full Bash (its entire privilege surface); core_commands.js:3137 — copyFile overwrites any existing agent file (idempotent replace, no skip-if-exists); hypaware-query/SKILL.md — sanctions regexp_extract(CAST(tool_args AS VARCHAR), …) over JSON_EXTRACT, baking a CLI quirk into guidance.

_{Generated by /decision-map. Advisory — directs attention, casts no verdict.}