hyp report CLI: member-facing reports plane commands, plus bundled report skills (LLP 0111)

[bgmcmullen]

Summary

• hyp report publish|list|get|delete (LLP 0111): member-facing CLI for the server's org-scoped reports plane, riding the existing --remote credential stack (target registry, 0600 store, silent OIDC refresh) with the reports endpoint derived from the one registered target URL. Directory publishes build a gzipped plain-ustar bundle; kind/period grammar and the entry-document rule fail fast client-side; publishes always send x-report-content-hash for retry safety; delete follows hyp purge's confirmation posture; a write 401 that survives the refresh retry names both remedies (expired session, or missing publisher role / publish token).
• LLP 0111 refinements: resolve two open questions in place — --period never defaults from the current date (only the report generator knows the coverage window), and the report group stays a REST command group rather than client verbs or client-side MCP tools (server-registered report_list/report_get MCP tools are the server corpus's to add). Matching @ref annotations at the two constraint sites.
• Bundle the report skills, rewritten on the new CLI: add hypaware-publish-report and hypaware-apply-report-changes to the claude and codex plugin workspaces. Publishing is now hyp report publish plus argument selection (the curl recipe survives only as a no-logged-in-hyp fallback); the apply skill reads a server's reports plane via hyp report list/get; codex copies swap Claude-specific tool mentions.
• Fix a latent attach break from Sync workspace skills: drop superseded report skills, update usage-report and report-to-html #320: that PR deleted the four superseded report-skill directories (adoption/improvement/security/spend) but left them in both plugin manifests and skill-register loops, and attach copyDirs every registered skill's sourceDir. Both lists now match the directories on disk.

Test plan

• npm test: report-commands suite (312 lines of new coverage in e618789) passes; the 8 failing tests on this branch (squirreling chunks is not async iterable in scanColumn/context-graph) fail identically on a clean checkout and are unrelated
• node --check on both plugin entrypoints; both hypaware.plugin.json files parse
• Skill content verified against the actual CLI surface in src/core/cli/report_commands.js (flags, error messages, confirmation posture)

🤖 Generated with Claude Code

[philcunliffe]

neutral review of report-cli (head af81cf5)

Verdict: changes-requested for one minor, documented-convention fix. The PR is otherwise correct, well-tested, and faithful to LLP 0111. Nothing here is a correctness, behavior, or security defect.

Finding 1 (minor, style/convention) - inline import('...') type

src/core/cli/report_commands.js:81

/** @type {import('node:fs').Stats} */
let stat

CLAUDE.md is explicit: "Never use inline import('...') types. Declare type imports at the top of the file with @import JSDoc comments, then reference the bare names." This file already follows that pattern for its other type (the @import { CommandRunContext } ... block at lines 24-26), so Stats is an internal inconsistency, and the repo's own tooling flags exactly this shape (src/core/plugin_doctor/diagnose.js:221).

What to change: add Stats to the existing top-of-file @import block and drop the inline form:

/**
 * @import { CommandRunContext } from '../../../hypaware-plugin-kernel-types.js'
 * @import { Stats } from 'node:fs'
 */
...
/** @type {Stats} */
let stat

(A prior instance exists at src/core/commands/purge.js:68; that is pre-existing and out of scope for this PR, noted only so the finding isn't read as new-in-this-PR-alone.)

What I verified as correct (not blocking)

• Bundle retry-safety holds. packUstarBundle shells out via execFile (arg vector, no shell => no injection through the path), and tar --format=ustar -cz is byte-deterministic across runs here (gzip stream mtime is 0, tar member mtimes are stable), so the x-report-content-hash dedup the LLP promises actually matches on a re-run.
• Auth flow is the shared --remote machinery verbatim. resolveReportsTarget + reportsRequest use resolveAccessJwt/attachWithRefresh/describeAuthRejection correctly (one-shot 401 refresh+retry, LLP 0058 D5). The write-401 dual-cause message fires only on a refreshable write credential, and env-override 401s route to the "re-login cannot fix an env override" path - both match the tests.
• Client-side fail-fast (kind/period grammar, entry-document rule, single-file extension) rejects before any bytes move, server stays authoritative - matches LLP 0111#fail-fast.
• report get path encoding encodes each segment and preserves separators deliberately; binary artifacts bypass the string-only kernel WriteStream type without a corrupting round-trip.
• report delete follows hyp purge's confirmation posture (TTY prompt, --yes for non-interactive), matching LLP 0104 / 0111#delete-confirm.
• deriveReportsEndpoint is a clean third sibling of deriveIdentityBase/deriveMcpEndpoint, strips a trailing /v1/mcp, and returns an unparseable URL unchanged (tested).
• The Sync workspace skills: drop superseded report skills, update usage-report and report-to-html #320 attach-break fix is complete and consistent. Both plugin manifests, both index.js skill-register loops, and the on-disk skill directories now agree (claude: 11 skills; codex: 9). The four superseded report-skill dirs are gone from all three places.
• Skill docs match the CLI surface (flags, error messages, confirmation posture). Codex and Claude publish/apply skills are identical after stripping client-name mentions.
• LLP 0111 @ref anchors all resolve (core-group, target, endpoint, bundle, fail-fast, delete-confirm, period-explicit, not-verbs, write-401); the two constraint sites (period-explicit, not-verbs) carry matching annotations.
• Style: no semicolons, no em dashes, root-anchored .js type-import specifiers, JSDoc-only types (apart from Finding 1).

Test coverage

test/core/report-commands.test.js (312 lines) meaningfully exercises the logic: endpoint derivation, single-file + folder-bundle publish (asserting the ustar magic and content hash), dedup-200, entry-doc/kind/extension rejection, quota error, --org forwarding, list rendering/--json/empty, unknown-target and env-override-401 paths, get-to-stdout/--output, and delete TTY/--yes gating. Good.

Local checks

Advisory only. node --test test/core/report-commands.test.js fails to load in this sandbox because the declared squirreling dependency is not installed here (it is reached transitively via the observability barrel and breaks every module that imports it); this is the pre-existing, unrelated failure documented in the PR body, not a defect in this change. CI (test + typecheck) is green and is the authority.

[philcunliffe]

changes-requested (1 minor finding). src/core/cli/report_commands.js:81 uses an inline import('node:fs').Stats type, which CLAUDE.md bans ("Never use inline import('...') types") and the repo's own plugin_doctor flags (diagnose.js:221). The file already has an @import block at lines 24-26, so add @import { Stats } from 'node:fs' there and use the bare Stats. Everything else - correctness, auth flow, bundle retry-safety, tests, LLP 0111 anchors, and the #320 manifest/disk skill reconciliation - checks out. See the review-record comment for the full pass. Marker:

[philcunliffe]

neutral review (round 2) - head 4d931ca - clean, approve

Round-1 finding: RESOLVED. The banned inline /** @type {import('node:fs').Stats} */ at src/core/cli/report_commands.js is gone. The head commit (4d931ca, "Type-import Stats at the top instead of an inline import() type") replaced it with an @import { Stats } from 'node:fs' block (line 25) and a bare /** @type {Stats} */ (line 82). grep -n "import(" src/core/cli/report_commands.js returns nothing.

Full review of the current head - no actionable findings

Reviewed git diff origin/master...4d931ca: the member-facing hyp report publish|list|get|delete group (report_commands.js, +519), deriveReportsEndpoint (credentials.js), the report command wiring (core_commands.js), the positionals/valueFlag export (remote_commands.js), tests (+312), LLP 0111, and the two bundled skills.

Correctness - verified:

• Auth/refresh flow rides the shared --remote stack verbatim (resolveAccessJwt + attachWithRefresh, one-shot 401 refresh+retry). Write-401 vs read-401 messaging is correctly split (LLP 0111#write-401): a surviving write 401 names both expiry and missing publisher scope; a 403 (org_mismatch) is correctly treated as non-refreshable and flows through as an ordinary error.
• URL/path encoding is correct: report get encodes each path segment via encodeURIComponent while preserving / separators (splits the artifact positional on / first); kind/period/id are each encoded. Query params use URL.searchParams.set.
• Endpoint derivation (deriveReportsEndpoint) is a faithful sibling of deriveMcpEndpoint: strips a trailing /v1/mcp, preserves path prefixes, appends /v1/reports, returns unparseable URLs unchanged. Covered by tests.
• Content-hash retry-safety (x-report-content-hash, sha256 of the uploaded body): I empirically confirmed tar --format=ustar -cz -C dir . is byte-deterministic across runs for unchanged input (GNU tar zeroes the gzip mtime header), so the dedup 200 path holds for bundles as well as single files - the LLP consequence is sound.
• Fail-fast validation (kind/period grammar, entry-document rule) rejects before any bytes move; tests assert calls.length === 0 on each reject path. Delete confirmation follows the hyp purge posture (TTY prompt, --yes required non-interactively).
• Style: JS no semicolons, no em dashes (U+2014), JSDoc types (no inline import() types anywhere in the diff), root-anchored .js type-import specifiers. All @ref LLP 0111#... anchors (core-group, target, endpoint, bundle, fail-fast, delete-confirm, period-explicit, not-verbs, write-401) exist in the doc.
• Skills/manifest: the two new skills' SKILL.md docs match the CLI grammar, flags, and error codes exactly. The manifest/index registrations drop the already-deleted report skills (no orphan directories remain) and add hypaware-publish-report + hypaware-apply-report-changes; registrations are consistent with on-disk skill directories.

Local checks (advisory): node --test test/core/report-commands.test.js -> 21/21 pass; tsc -p tsconfig.json --noEmit -> clean. CI is authoritative and green.

Nothing actionable. No fixes pushed.

[philcunliffe]

neutral approves PR #338 at head 4d931ca: mergeable, CI green, reviewed clean. Round-1 finding (inline import type) is resolved in your push. Held for you to merge; neutral never merges or readies a contributor's PR.