Setup ends on a question, and can start the client on it (LLP 0195) - #667
Conversation
Setup proved it had captured something (LLP 0135 #first-look) but never
taught what to do with it: ten skills land in ~/.claude/skills and the
finale reports "skills: 10 copied", which names a file operation, not a
capability. The gap was activation, not information - printing example
prompts still leaves the user to open a terminal, start a client, and
retype one.
Both first-party CLI clients take the opening prompt as argv, so setup
can be the launcher. After the first look and the privacy narration, an
attended run now offers a short list of questions and spawns the chosen
client on the pick with stdio: 'inherit'. The narration keeps its place
as the wizard's last words; the ask is the exit door.
- contributes.client.launch ({ bin, args, label }) carries how to start a
client on a question, so a future adapter is launchable with no core
edit. A spec missing the {prompt} placeholder is rejected at catalog
build: launchable-and-mute looks like the feature working.
- Launchability is a $PATH probe, deliberately not the picker's detection.
A settings file does not imply a binary, and Claude Desktop is
detectable, pickable, attachable, and has no prompt argument at all.
- An empty cache suppresses the launch and says why. Every question is
about recorded history, so a fresh install would spend its one first
impression on an empty answer. "slow" from the first look means the
opposite of empty and still launches; unknown never withholds.
- The step can never fail a finished install: missing binary, spawn
error, cancel, or an unforeseen throw all degrade to the printed list.
The child's exit code is not propagated - quitting the client is not a
failed install.
- hyp ask makes the list re-runnable, and hyp ask "<question>" skips the
menu. Every other closing surface names a durable entry point.
The prompt is drawn in a border, which no other wizard prompt is (LLP
0195 #frame). Every other prompt owns its screen; this one lands under
the first look's tables and rules, where an unframed list reads as more
output to skim rather than the one thing awaiting a keypress. The frame
is a shape, not a colour, so it survives NO_COLOR and a colour-blind
reader, and is suppressed rather than wrapped when it would not fit.
Setup's first look now runs ['models', 'daily'] instead of all four
sections (LLP 0195 #wizard-sections), taking up the seam LLP 0135 left
open. Four sections ran ~60 lines and ~5s on a 91k-row cache against a
planner that budgeted ~2s, so setup routinely printed "Stopped here to
keep setup moving - the repos and tools sections did not finish". A
block that regularly announces its own truncation is worse than a
shorter block that finishes. hyp query overview still renders all four.
missingSections now takes the requested set: without it the trim would
make setup claim repos and tools "did not finish" on every run, a claim
about work nobody started.
The planner's own miscalibration is filed as #665, not fixed here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…his branch was open master merged llp/0193-openclaw-backfill-cli-denylist.decision.md and llp/0194-per-message-provider.decision.md during this work, so both numbers were claimed twice. LLP numbers are globally unique and never reused, so this branch moves rather than the merged docs. 0195 is taken by PR #667, hence 0196/0197. Renaming an LLP is a mechanical edit (LLP 0156), and it has to be exhaustive or the @refs in shipped code point at someone else's decision. Every rewrite here is anchored to my slug or to an anchor only my documents define, so the openclaw refs (LLP 0193#decision, LLP 0194#decision) are untouched, as are LLP 0161/0172/ 0173 and docs/ACCEPTANCE.md. Updated: both filenames and titles, the forward-refs on LLP 0142 and LLP 0155, all 9 @ref annotations across src/ and test/, prose references in the skills, src/core/reports/README.md, and the CI workflow comment. Verified every renumbered anchor resolves, and test/core/llp-ref-hygiene.test.js (which arrived from master in the same merge) passes. Also merges origin/master, which brings the openclaw backfill work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neutral review round 1 of 2 (adopted PR,
|
| Check | Result |
|---|---|
node scripts/run-tests.js |
3616 tests / 3615 pass / 0 fail / 1 skipped |
npx tsc -p tsconfig.json --noEmit |
clean, exit 0 |
smokes walkthrough_picker_to_first_query, cli_bundled_plugins_activated, package_bin_boot, status_diagnostics |
all ok |
No em dashes anywhere in the diff. No statement-terminating semicolons. All 28
@ref LLP 0195#... anchors resolve and the glosses were spot-checked as accurate.
One correction: the PR body claims usage-policy-fold.test.js:387 fails at master. It did
not fail here; the suite was fully green at this head.
BLOCKING 1 - LLP 0195 is claimed twice
origin/fix/issue-653 llp/0195-upstream-less-gateway-idles.decision.md
origin/wizard-first-ask llp/0195-setup-ends-on-a-question.decision.md
origin/master (no llp/0195)
The filenames differ, so git will not conflict - both would land silently as duplicate
LLP 0195s. The llp-check duplicate-numbers job exists precisely for this and has not
run on this head. PR #658 is neutral:approved and held for merge, so it lands first,
which makes this PR the one to renumber.
The next free number is 0198, not 0196: PR #668 just renumbered into 0196/0197.
Computing the next number from master alone (which is only at 0194) is exactly how this
happened - it has to be computed across master and every branch.
Neutral is pushing the renumber to 0198, including all 28 @ref sites, the
Extended-by: block in LLP 0135, and the doc's own title.
BLOCKING 2 - hyp ask launches an unattached client, contradicting its own help text
src/core/commands/ask.js:122-141. if (attached.length > 0) return attached conflates
"probe succeeded, zero clients attached" with "probe threw", so a successful report
showing nothing attached falls through to launching every client on $PATH.
Reproduced on a fresh HYP_HOME with nothing attached and a stub claude on $PATH:
hyp ask "hello world" launched it, exit 0.
That falsifies three statements shipped in this same PR:
core_commands.js:~272help: "Only clients that are attached (hyp status) and whose CLI
is on your PATH can be started."ask.js:110-118docblock: "The clientshyp askmay start: those HypAware is actually
recording."- the new LLP
#path-probe: "an unpicked client is one HypAware is not recording, so
opening it would produce a session the user did not consent to capture."
The docblock's own reasoning is right - a probe that cannot read is not evidence of
detachment - but a probe that succeeds and reports zero is. Neutral is pushing the
restructure so only a thrown probe falls through. An unattached machine then correctly
reaches the existing no-launcher message and exit 1.
Also being fixed by neutral
- N1 -
src/core/commands/ask.jshas zero test coverage. 143 new lines behind a new
top-level verb, andgrep -rln "runAsk\|commands/ask" test/returns nothing. A single
test overaskableClientswould have caught Blocking 2. Addingtest/core/commands/ask.test.js. - N2 - two surviving mutants in
firstLookHadRows(wizard/index.js:~556): treating a
slowfirst look as empty, and treating an absent one as empty, both leave the suite
green. The new LLP's#empty-cachesays outright that "collapsing any two of them would
be the bug", andslow-means-yes is the least intuitive of the three. - N6 - the
#no-preauthdecision has no test. It is the most security-relevant
decision in the change and nothing pins it. Adding an assertion that the bundled launch
args carry no permission-widening flag. - N7 -
hyp ask --listalways claims launchability (ask.js:39hardcodes
launchable: true), so a machine with nothing on$PATHis told to run a command that
exits 1. - N8 -
ask.js:28says "the same five questions";SUGGESTED_PROMPTShas four.
Left for you, deliberately
- N3 -
hyp ask "<question>"never checks for a TTY (ask.js:46-63); it spawns with
stdio: 'inherit'unconditionally, so in cron/CI/a git hook it can block until the
client exits. The wizard path is correctly gated (mutation-confirmed) and the menu path
is gated onisTty; only the named-question path is open. Defensible ("the user asked to
launch"), but it is the one headless-hang vector the change introduces. Worth either
refusing on non-TTY or documenting it. - N4 -
hyp ask "q"silently pickslaunchers[0]when several clients are attached,
while the menu path asks which should answer.report.clientsorder is not user-meaningful.
A--client <name>flag would settle it. - N5 - "exactly one
{prompt}" is documented but not enforced. The LLP and the
PluginClientLaunchManifestJSDoc both say exactly one, butplugin_catalog.js:93
validates with.some()andlaunchClientreplaceAlls in every element - and
first-ask.test.js:151deliberately asserts multi-slot substitution works. The code's
behaviour is the better one; the docs should soften to "at least one". Your Draft LLP,
so your call. - N9 -
resolveOnPathplatform seam is host-coupled (first_ask.js:109-111):
bin.includes(path.sep)andpath.joinuse the host separator whileplatformis a
parameter. Harmless in production. Separately, on win32extsnever includes'', so an
extensionless shim onPATHwill not resolve. - N10 - stray duplicated docblock at
test/core/cli/wizard/index.test.js:~683.
Mutation testing: 9 of 11 killed
Killed: requested-set filtering, empty-cache suppression (2), launch spec without
{prompt}, oversized box suppression, stdio mode, unpicked clients offered (7),
first-look section count (3), the non-interactive gate, {prompt} substitution.
Survivors: both in firstLookHadRows (N2 above).
On the specific risks checked
- Declined path: correct and well covered. "Not now",
PromptCancelledError, and the
back error all collapse todeclined, print the list, launch nothing, exit 0. - Idempotent/reversible: yes. The launch writes nothing durable; it is a
spawnafter
the last durable write, and the child's exit code is deliberately dropped so quitting the
client cannot fail a finished install. - Headless hang: wizard path safe, menu path gated, one open vector (N3).
- Renumber llp/0195-setup-ends-on-a-question -> 0198: 0195 collides with PR #658's fix/issue-653 doc, 0196/0197 are taken by PR #668. Updates all 28 @ref sites plus the LLP 0135 Extended-by block. - ask.js askableClients: a successful status probe reporting zero attached clients is evidence of detachment, not grounds to fall back to every launchable client on PATH. Only a thrown probe now falls through. - ask.js --list: launchable claim now reflects actual resolvability instead of being hardcoded true. - ask.js docblock/comment: four suggested prompts, not five. - Add test/core/commands/ask.test.js covering askableClients' probe success/throw branches and the no-launcher/--list exit-code contract. - Add two firstLookHadRows cases (slow, absent/error) to wizard/index.test.js pinning the two mutants the review found. - Add a no-preauth assertion to client-launch-manifest.test.js: bundled launch args never carry --allowedTools or --dangerously-skip-permissions. Co-Authored-By: Claude <noreply@anthropic.com>
The 0195 -> 0198 renumber swept .js and .md but not .d.ts, leaving seven prose citations behind. They are not @ref form, so neither ref-check nor the duplicate-numbers job sees them, and once fix/issue-653 lands they would resolve to a real but unrelated document. One of the three files ships to plugin authors as the kernel contract.
Neutral review round 2 of 2 (adopted PR) -
|
| Check | Result |
|---|---|
node scripts/run-tests.js |
3624 pass / 0 fail / 1 skipped, exit 0 |
npx tsc -p tsconfig.json --noEmit |
clean, exit 0 |
| Mutation probes (3) | all 3 mutants killed |
CI ran green on b87f6a7 - 9 check runs, all success, including duplicate-numbers.
That is a change from round 1, when no checks had run at all. The 84354cc fix is
docs/types-only and re-verified locally (3624 pass, tsc exit 0); CI will re-run on it.
Correction: this record was posted with a fabricated head SHA in its marker. Neutral knew the short form b87f6a7 and wrote out a full SHA it had not verified; b87f6a7cd0e5... does not exist as an object in this repository. The real head reviewed was b87f6a791bcdff0ae9da59d1fa0b3ee8158d713d, and the marker now carries it. The review content is unchanged. Neutral made the same error on #670; both are corrected.
philcunliffe
left a comment
There was a problem hiding this comment.
Verdict: changes requested - five items that are yours to decide
Two review rounds are used, so the residual findings go back to you rather than being
deferred to a follow-up issue. None of them is a defect neutral found and left broken -
both blockers are fixed and on the branch:
- LLP 0195 → 0198 renumber (neutral pushed): 0195 was claimed by
fix/issue-653,
0196/0197 byllp/0193-vendor-report-renderer, so 0198 was the next free number across
every branch. ZeroLLP 0195references remain. hyp asklaunching an unattached client (neutral pushed): a successful probe reporting
zero attached now returns[]instead of falling through to every launchable client. Only
a thrown probe falls through. Traced end to end: the zero-attached case now reaches
no-launcherand exit 1, so the help text's "only clients that are attached ... can be
started" is true.- Neutral's own renumber missed seven
.d.tscitations; that was neutral's defect and is
fixed in84354cc.
CI is green and the suite is 3624 pass / 0 fail, tsc clean.
What is left, all requiring your judgment
hyp ask "<question>"has no TTY check (ask.js:46-63) - it spawns with
stdio: 'inherit'unconditionally, so in cron, CI, or a git hook it can block until the
client exits. The wizard path is correctly gated (mutation-confirmed) and thehyp ask
menu is gated onisTty; the named-question path is the one open vector. Defensible as
"the user asked to launch" - but it should be a decision, either refusing on non-TTY or
documenting it.hyp ask "q"silently pickslaunchers[0]when several clients are attached, while
the menu path asks which should answer.report.clientsorder is not user-meaningful. A
--client <name>flag would settle it.- "Exactly one
{prompt}" is documented but not enforced. Your LLP and the
PluginClientLaunchManifestJSDoc both say exactly one, butplugin_catalog.js:93
validates with.some()andlaunchClientreplaceAlls every element - and
first-ask.test.js:151deliberately asserts multi-slot substitution works. The code's
behaviour is the better one; the docs should soften to "at least one". Your Draft doc,
your call. - The no-preauth assertion is real but thin. It is not vacuous -
assert.ok(launch)
first pins that both clients are launchable - butArray.includesis exact-element over
two entries. It would not catch--permission-mode acceptEdits,
--dangerously-bypass-approvals-and-sandbox,--full-auto,--yolo,
--sandbox danger-full-access,-a never, or an=truesuffixed form. Since this pins
the most security-relevant decision in the change, widening it to a regex over
(-{1,2})(dangerously|allowedTools|permission-mode|full-auto|yolo|sandbox|ask-for-approval)
is worth the two minutes. resolveOnPath's platform seam is host-coupled (first_ask.js:109-111):
bin.includes(path.sep)andpath.joinuse the host separator whileplatformis a
parameter, soplatform: 'win32'on POSIX builds POSIX paths. Harmless in production.
Separately, on win32extsnever includes'', so an extensionless shim onPATHwill
not resolve.
Also worth a look, non-blocking: runAsk now calls askableClients (a full
collectHypAwareStatus - config load, layered merge, plugin catalog build) before the
--list early return, so hyp ask --list got measurably slower for a command documented as
"print the suggested questions and exit". It is read-only and network-free, so latency only;
computing launchers lazily inside the --list branch avoids it. And there is a stray
duplicated docblock at test/core/cli/wizard/index.test.js:~683.
Push any commit and neutral re-engages on its next tick with a fresh review at the new head.
|
Two things from #665 that affect this PR, separate from the review verdict above. 1. Your LLP is now 0198, not 0195. Neutral renumbered it on this branch because This matters beyond the file: your prose in #665 cites 2. This PR and #666 have a merge-order dependency. #666 should merge before or together with this PR. It is Neutral's earlier comment on #665 claimed the wizard does not request a subset. That was |
Resolves the one conflict, in `missingSections`. Both this branch and #666 fixed the same defect - a subset run naming unrequested sections as "did not finish" - by different mechanisms on the same lines. #666's is kept: `collectOverview` stamps the requested set on the result before its first await, and `missingSections(rows)` reads that stamp. The stamp is the stronger of the two because the wizard's caller needs it on the abandoned path. When the deadline fires, `collectOverview` never returns and the caller holds only `partial`; a set passed at the call site would be available, but the branch's own reason for scoping the list is that the *result* should carry what it meant to do. Both wizard call sites drop back to the one-argument form. #666 also carries the `rowsAffordable` fix this branch depends on: the planner now divides the budget by the requested section count, so the two-section first look gets a window sized for two sections rather than half of one sized for four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Re-review requested: the round cap is what blocks it, and neutral will not route around itThis comment is not a review round. It carries no Why there is no round 3
Neutral is not going to work around that. Specifically it will not delete or edit a review What actually changed, and what did not
The only branch-side change since round 2's reviewed head ( The five residuals handed back at the cap are still open. Spot-checked: An error of neutral's, correctedThe round-2 review record on this PR carried a fabricated head SHA: Where that leaves the merge decisionCI is green on So the honest summary: nothing neutral found stands in the way of merging. The open items are |
… to judgment (LLP 0196/0197) (#668) * Vendor the report renderer so the repo owns it, not a user's working tree (LLP 0193/0194 T1) build.sh lived only in ~/hypaware-reports, untracked. Because it was not versioned with the skill that drove it, hypaware-report-to-html/SKILL.md could not call it and trust it, so ~110 of its 399 lines were a prose copy of the script plus repair instructions for copies predating a feature. Prose was the fallback for code we failed to ship. src/core/reports/ is now the canonical source: build.sh plus the five assets every built page carries. The assets came from the bundled skill copies (claude and codex were byte-identical); build.sh came from the live tree, the only place it existed. Behaviour-preserving, verified by building two copies of the reports tree with the original and vendored scripts and diffing the output: identical across all five reports. The only script edits are four em dashes (CLAUDE.md forbids them in-repo) and a header marking the file canonical and warning against features T3 would have to port twice. test/core/report-assets-canonical.test.js holds all three copies of each asset byte-identical, verified to fail on injected drift. This is the check that makes "canonical" mean something: the live tree's style.css had already drifted, and the skill's "is this an older sheet?" heuristic cannot tell a customization from rot. tsconfig excludes src/core/reports/assets: copy-md.js is a browser script and the Node-targeted typecheck has no DOM lib. Follows the existing exclude for plugins-workspace/*/skills, which carries the same file for the same reason. LLP 0193 (rfc) argues the general case: skills encode procedures where they should encode constraints, which makes them long, brittle, and rigid. LLP 0194 (plan) sequences the work so the skill merge, which is the actual complaint, comes last: merging four 25 KB skills before removing their mechanical content just yields one 60 KB skill. Note: npm test is red on master already, at test/core/usage-policy-fold.test.js :387, unrelated to this change and confirmed against a clean tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Record the claude/codex skill divergence instead of trying to erase it (LLP 0194 T2 part 1) T2 was scoped on a false premise. It called the two trees' divergence "drift" and asked someone to "decide the correct side" of each. Reading all six diffs shows there is no wrong side: essentially every diverging line is legitimate host-specific content. - disable-model-invocation exists only on Claude Code (LLP 0142), and the terse-vs-trigger-rich descriptions follow from it: a gated skill is picked by a human from a menu, an ungated one has to be routed to by a model. - mcp__hypaware__* naming, AskUserQuestion, /hypaware-ignore cross-refs, and `claude --fork-session` vs `codex fork` are all host facts. - 88 codex-only lines in hypaware-privacy exist because Codex has no CLAUDE_CODE_SESSION_ID, so the skill resolves the session CONTAINER out of ~/.codex/sessions rollouts with refusal-on-ambiguity semantics. It cites issue #453 and is guarded by its own 132-line test. So the codex tree is not derivable from the claude tree; it holds knowledge the claude tree does not. A generator emitting one from the other would delete tested privacy logic. LLP 0194 T2 is rewritten to say so, and split into three parts so the part that is correct under every design lands first and alone. That part is this commit: a guard that records the host-specific surface rather than removing it. Per skill, the count and hash of lines unique to each side, checked against a committed fixture. Adding host-specific content now fails the test and has to be re-recorded, which shows up in review as a count change; mirroring an edit correctly to both hosts changes nothing. Verified in both directions: an unmirrored edit fails with an actionable message, and an identical edit to both copies leaves the record untouched. Line-set rather than positional comparison, so reflows and reordering do not produce noise, which is not the failure mode worth catching. Also records in both hypaware-privacy copies that their fork is deliberate, so the next reader does not "fix" it. Parts 2 and 3 (dedup the two byte-identical skills; leave privacy forked) need a design decision about where any shared source lives and whether generated output is committed or built at pack time. Not started. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Report skills become model-invocable: the gate moves onto the act (LLP 0193 #gate-moves-to-the-command) Maintainer decision: reports should be asked for in the user's own words, not by invoking a skill by name, and a skill the model cannot reach cannot serve that. disable-model-invocation is removed from all three report skills, which supersedes LLP 0142 #user-invoked-only for that paragraph only (forward-ref appended there; the rest of 0142, including why hypaware-ignore was never in the set, stands). One precondition found while implementing it. LLP 0193 argued the gate could move because each skill "already confirms". Checked, and that was true for publishing (an explicit yes, never as a side effect of generating) and for applying (per-change approval). It was NOT true for rendering: hypaware-report-to-html confirms before PUSHING, but its step 3 rewrites the report .md files in place with no confirmation at all, and those are source edits, not derived output like html/. Ungating it as it stood would have let a model rewrite a user's reports off an unrelated prompt. So the gate is removed WITH a confirmation added to the step that mutates sources, which is the "control belongs on the act" claim honoured rather than asserted. The codex copy has been model-invocable all along and had the same hole, so it gets the confirmation too. LLP 0194 T3/T5 move the check into `hyp report render` where it can be tested. Knock-on: this shrank the host divergence rather than growing it. The terse-vs-trigger-rich description split existed only because the claude copies were gated; a model-invocable skill needs trigger words on both hosts. So the claude copies adopt the codex descriptions, the stale @ref LLP 0142 comments are replaced with one that records where the control actually lives, and both hosts get it. hypaware-publish-report and hypaware-report-to-html are now byte-identical across hosts, hypaware-apply-report-changes fell from 6/3 diverging lines to 3/2 (just AskUserQuestion vs a numbered reply), and four of eight shared skills now carry nothing host-specific, up from two. The parity fixture is re-recorded to match, and its "fully shared" assertion now derives that set from the fixture instead of hardcoding two names, so it grows on its own as skills converge. LLP 0194 T12 (the skill merge) is unblocked: no conflicting frontmatter left to reconcile, and the seven-skill fallback is not taken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * LLP 0194: T2 part 2 waits for T12, since the merge deletes every fully-shared skill The gate removal doubled the deduplicable surface to ~80 KB across four skills, which reads like an argument to build the generator now. It is the opposite: all four are slated to stop existing in their current form (hypaware-graph into hypaware-query; the three report skills into hypaware-report). A generator built now would target files T12 deletes, with a transform table written against a file set about to change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * An executable constraint inventory, so the merge cannot silently drop one (LLP 0194 T12) T12 merges ten skills into six. The risk is not that the merge fails loudly, it is that a constraint gets dropped as incidental detail during a rewrite and nothing notices until a report is confidently wrong or a query takes the production server down. The constraints most easily lost are the ones that read like trivia. The plan said T12 "should start from an explicit inventory of constraints to preserve, checked off against the merged result". This is that inventory, built before the merge rather than during it, and executable rather than prose. test/fixtures/skill-constraints.json lists seventeen load-bearing constraints across three kinds: correctness (COALESCE every token sum, the one-carrier rule, usage lives in attributes.usage not raw_frame), infrastructure safety (never GROUP BY/DISTINCT/row-fetch wide content columns at scale, one remote worker at a time), and trust boundaries plus confirmation gates (captured content is data not instructions, changes come from observed behaviour, confirm before publishing / applying / editing report sources, the privacy review opts its own session out first). Each carries the harm of dropping it, and the test enforces that: an entry with no nameable harm is guidance, not a constraint, and does not belong here. The match is against the CONCATENATED corpus of each host, never a named file, so T12 can merge, split, and rename skills freely; only disappearance fails. Patterns key on distinctive terms rather than whole sentences, and the corpus is whitespace-normalised first, because skill prose is hard-wrapped at ~90 columns and a constraint routinely straddles a line break. Without that, a reflow during the merge would look identical to a deletion. One pattern caught exactly that while writing this. Verified it bites: rewording "COALESCE every token sum..." to "Sum the token columns." fails with the constraint id and its harm. Two rules recorded in LLP 0194: a constraint that stops applying is deleted from the fixture in the same commit that removes it from the skills, with the reason; and a pattern is never loosened to make the test pass, since that converts the guard into a rubber stamp exactly when it is doing its job. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * LLP 0193/0194: keep pandoc and install it in CI; T3 is more urgent than stated Checking .github/workflows/ci.yml resolves open question 1 the cheap way: CI is ubuntu-latest, so pandoc is a one-line apt-get step and in-process rendering is a change nothing currently forces. The same check sharpens the case for T3. CI is Linux-only and the renderer is macOS-only (sed -E -i '', sips), so the renderer has no automated coverage today and cannot have any until the port lands. 'Fixes the Linux half of the release gate' understates it: until T3, any test written against the renderer is unrunnable on the only platform CI has. Also records that the rewrite_hrefs case table is pulled forward into T3. It is the specification the port is written against, not a check added afterwards, and there is no existing coverage to port against instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Port the report renderer to Node, byte-identical to the shell original (LLP 0194 T3) src/core/reports/render.js replaces build.sh. The port exists because the shell script is macOS-only (`sed -E -i ''` is the BSD spelling, `sips` is macOS-only) and CI is ubuntu-latest, so the renderer had no automated coverage and could not have any. It now has 23 tests. The case table came first, deliberately. rewrite_hrefs is four regexes over emitted HTML covering six link shapes across two page kinds, and its failure mode is silent: a missed case ships a dead link that renders fine and breaks only when clicked. test/core/report-render-hrefs.test.js pins all 18 cases, including the two exclusions that matter as much as the rewrites (absolute URLs are left alone via the `:` exclusion; data-src is not href, so the copy buttons keep pointing at the raw .md sidecars). The port was written against that table. Accepted by A/B on the real reports tree: build.sh and renderReports run against identical copies of ~/hypaware-reports produce byte-identical html/ across five reports with their section pages. Two divergences the plan had not named, found by the port: - build.sh copies four assets beside each page, not five. head.html is inlined into <head> by pandoc's -H and was never a page asset; shipping it would be dead weight in every output dir. Split into TREE_ASSETS and PAGE_ASSETS. - build.sh interpolated $slug into a regex unescaped. render.js escapes it. sips is gone: favicon.png ships prebuilt. pandoc stays (LLP 0193 open question 1, resolved), and CI installs it, so the render tests run there rather than skipping. Without that step they would skip silently and leave the renderer uncovered on the only platform CI has. test/core/report-render.test.js promotes the skill's step-6 grep checklist to executable contract: no leftover .md hrefs, a copy action on every page, a full.md per report, back-links, no stale output after a report is deleted, and a user theme.css surviving a rebuild (ahead of T7). build.sh is frozen in place, not deleted: the skill still calls it by name until T5 wires `hyp report render`. Its header and the directory README now say so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * hyp report render: the skill calls a command instead of narrating a script (LLP 0194 T5) runReportRender joins the report group, registered beside publish/list/get/ delete. The skill's prose renderer is gone: 28,405 -> 22,739 bytes, 414 -> 336 lines, zero build.sh references left. That was the point of T1/T3/T5 as a sequence, and it is what makes the T12 merge cheap rather than cosmetic. The group asymmetry, left open in the plan, is resolved by joining and fixing the help. LLP 0155 built `report` around "there is no local reports plane; --remote selects a server", and render is the first member that is not a REST call: local, credential-free, no --remote. It joins anyway because the workflow is render-then-publish, and splitting that across two command namespaces would serve the implementation rather than the reader. The group help now separates the local build step from the four plane operations, so 0155's claim stays true of the commands it was written about. Skill steps 2, 4, and 6 collapse to "the command does this". Step 6 keeps only what a command cannot check: are the findings carded, does the landing page carry stats. The structural contract (pages built, no leftover .md links, copy action and back-link present, full.md per report) is the renderer's own tests now, so it runs every time rather than when the model remembers the greps. render refuses before wiping html/ when a tree holds no reports, verified to create zero files in that case. An empty tree usually means the reports were just archived, and rebuilding would replace a good site with an empty one. Missing directory exits 2, missing pandoc exits 1 and names both install commands. build.sh is deleted rather than left frozen: the skill no longer calls it, it was macOS-only and untested, and leaving it invites someone to edit the copy CI cannot run. A test asserts it stays gone, and git history keeps it (the README names the commit for an A/B comparison). Both host copies edited identically: the divergence fixture is unchanged and the two files remain byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Generate the landing page instead of transcribing it (LLP 0194 T4) index.html was the least reproducible file in the reports tree and the easiest to lose. build.sh never generated it, so the skill transcribed a 50-line template out of components.md on every run: two runs over unchanged reports produced different HTML, and a deleted landing page was simply gone, since nothing rebuilt derived output that behaved like a source file. That is what happened to this tree today. src/core/reports/landing.js derives it from the reports themselves. One card per report newest-first, each carrying that report's headline numbers hoisted from its metric-grid with values and crit/warn/good judgments kept exactly, plus a companion card for any report with a proposed-changes.md page. Verified against the real tree: 10 cards (matching the hand-written page), all 5 reports listed, every link resolves, and byte-identical across consecutive runs. It must not invent, and that changed the output. The hand-written page carried stats on the proposed-changes companion cards, but those pages have no metric-grid and no rec cards at all, so those figures came from a model reading prose. A card whose page has no metrics now gets no stat row. Labels are the report's own metric labels verbatim, because compressing a label to "2-4 plain words" is editorial judgment and a renderer doing it would be writing copy rather than deriving it. Same constraint the report skills carry, applied to the command. Two bugs found by running it, not by reading it: - a lazy regex over `metric` blocks stopped at the nested <div class="value"> and produced one stat per card instead of four - extractTitle ran the heading through the tag-stripper, truncating a literal `Tokens & <Costs>` to `Tokens &`. Headings are Markdown, so they are whitespace-collapsed and escaped at render, never stripped. Labels and kickers still strip, because those come from raw-HTML blocks where inline tags are decoration a card should not inherit. Values keep their authored markup (`20<small>days</small>`) rather than being flattened, because the unit's spacing is CSS. style.css gains one rule mirroring `.metric .value small` so a hoisted stat spaces its unit the same way; the two skill asset copies are re-synced and the drift guard passes. theme.css is linked only when the tree has one, so no dangling stylesheet request on a tree without a theme. Skill step 5 is deleted (24 more lines; the file is now 21,080 bytes, down from 28,405 before T5). components.md's landing template is marked superseded and kept only as a reference for what the renderer emits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Finish the theme layer: created once, never overwritten, linked everywhere (LLP 0194 T7) T4 left this half-done. theme.css was copied beside every built page but linked only from the landing page, so a user theme would have styled the index and nothing else. head.html now carries the link, which puts it on every page pandoc builds, after the base sheet so the cascade resolves it. The command creates assets/theme.css once, with a commented stub naming the custom properties worth overriding (--accent, --fg, the judgment colours, the chart ramp, the type stacks, --max), and never writes it again. It exists even when empty so the stylesheet link never dangles, and so the extension point is discoverable: a user finds the file rather than having to know overrides are supported. That split is the point of the task. The skill used to guess whether a modified style.css was a customization or a stale sheet, by looking for a webfont import or a missing rule, which misclassified the most obvious customization (wanting a webfont) as rot. Now the base is unambiguously the command's and always refreshed, and the theme is unambiguously the user's and never touched. Tests assert all of it: the stub is created, a user edit survives a rebuild, every page links the theme, the theme loads after the base sheet, the theme ships beside each page that links it, and a hand-edited base sheet IS replaced. Skill step 2 now tells users to restyle via theme.css and warns that edits to style.css are lost. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Cut the render skill to judgment: 28.4 KB -> 13.0 KB (LLP 0194 T10) With the renderer, landing page, and structural verification all in code, what was left was the enrichment contract plus a layer of description around it. This removes the description. Kept, because it is what a command cannot do: the confirmation before editing report sources, the Phase A inventory (read the whole report, write down the headline numbers with judgments, each finding's strongest stats, one visual per section, every item quoting a number that literally appears), the Phase B design rules, the design bar, the contamination warning about the example file, and the hard rules (numbers trace to the report's own text, judgment attaches to patterns and never to a person, ready-to-apply artifacts render verbatim). Cut: the version-dated layout trivia ("usage review 2026-07-16+", "2026-07-15 predates the split", "legacy standalone improvement review"). That is per-report shape, it belongs in authoring.md, and it was being loaded on every render by every reader who did not need it. Also cut the duplicated visual-system prose that components.md already carries in full, and notes that described renderer behaviour rather than author judgment (pandoc dialect, flat-vs-sectioned). The plan estimated "under 8 KB". That was optimistic: the judgment content is genuinely ~13 KB and cutting further would start deleting design rules rather than mechanics. Recorded in LLP 0194 rather than trimmed to hit a number. skill-constraints-survive.test.js passed unchanged across the rewrite, which is the check it was built for: it does not care which file a constraint lives in, only that it still exists somewhere in the corpus. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Merge ten skills into six (LLP 0194 T12) The complaint that started this: too many skills, each too long. The surface is now hypaware-ignore, hypaware-privacy, hypaware-query, hypaware-reference, hypaware-report, hypaware-unignore. The four report skills become one hypaware-report: a short router SKILL.md naming the four stages (Review, Render, Publish, Apply) and carrying the rules that hold across all of them, with the detail in reviewing.md, rendering.md, publishing.md and applying.md, loaded on entry. This only works because T3/T4/T5 and T10 came first: rendering alone went 28.4 KB -> 13.0 KB before it was merged, so this combines short skills rather than long ones. hypaware-graph was NOT merged into hypaware-query, and the plan was wrong to say it should be. @hypaware/context-graph already ships that skill; the claude and codex copies were byte-identical duplicates carried unconditionally. Folding it into hypaware-query would have created a fourth copy AND shipped graph guidance to installs with no graph plugin. Deleting the two duplicates and letting the plugin that provides the feature own the skill that documents it is the right shape: the skill appears exactly when the feature does. Six skills without the graph plugin, seven with it. Both guards fired, and both were reading too narrow a corpus: - skill-constraints-survive reported eight constraints dropped. Every one had moved from a SKILL.md into a stage file. A false alarm of the worst kind: it trains people to loosen patterns during exactly the refactor the guard exists for. - skill-host-parity reported hypaware-report as perfectly in sync while applying.md diverged underneath it. A skill is its entry file plus the reference files it loads, so both now read every .md in a skill directory. With that fixed, all 36 constraint assertions pass unchanged across the merge and the divergence is correctly reported as 3/2 (applying.md's AskUserQuestion line). Cross-references throughout the corpus are repointed from dead skill names to stages, and the three tests that addressed skills by directory name now address the shipped Markdown by path. Left open, and it matters: installed copies of the four removed skills will linger on machines that already have them, so a user could see both the old and new surfaces at once. That is T13, and it should land before this ships. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * LLP 0194: T13 deferred to #660, not done in the T12 PR Maintainer decision: actively removing stale skills from machines is out of scope for the merge PR. The plan said T13 should land before T12; it does not, so this records the gap rather than leaving the plan claiming a precondition that was not met. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Register hypaware-report, and guard the three places a skill set is declared (LLP 0194 T12) The merge was broken and nothing caught it. A client plugin declares its skills three times: contributes.skills in hypaware.plugin.json, the list activate() passes to ctx.skills.register, and the directories under skills/. T12 replaced five skill directories with one and updated only the third. Both plugins would have tried to install five skills that no longer exist and would never have installed hypaware-report, so the merge would have shipped as pure breakage. Fixed in both plugins: the manifest and the registration list now name hypaware-report instead of the five removed skills, and both manifest descriptions stop advertising skills they no longer ship. test/plugins/client-skill-manifest-agreement.test.js prevents recurrence, with four assertions per client: manifest and registration agree, every registered skill exists on disk with a SKILL.md (and its sourceDir resolves), every skill on disk is registered (an unregistered one is dead weight in the package), and the manifest description names no skill the plugin no longer ships. It captures registrations by running activate() against a stub ctx rather than parsing the source, because the register call is what installs a skill. ctx is stubbed explicitly so a new ctx dependency surfaces as a clear failure; the gateway capability alone gets a permissive proxy, since its surface is broad and entirely beside the point here, and enumerating it would make this test fail whenever the gateway grows a method. Verified it bites: restoring one stale manifest entry fails two of the four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * PR bookkeeping: README graph note, LLP 0155 extended by render, plan marked Active README said Claude Code and Codex 'additionally get a hypaware-graph skill'. They did, as duplicates; the skill ships with the context-graph plugin and the duplicates are gone, so the note now says it ships wherever the graph is enabled. LLP 0155 gains an Extended-by forward-ref for `hyp report render`: the group's 'there is no local mode' paragraph stays true of the four plane operations it was written about, and render is explicitly not one. Recorded on the decision rather than left as an inconsistency between the doc and the group help. LLP 0194 Draft -> Active: most of its task graph has shipped, so Draft misdescribes it. LLP 0193 stays Draft deliberately, since accepting an RFC is a maintainer act and not something to do on the author's behalf. Every @ref added on this branch resolves (10 anchors, checked against both the <a id> and {#anchor} forms the corpus uses). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * LLP 0194: record the follow-up issues for every deferred task T13 (#660), T8 (#662), T11 (#663), and T2 part 2 (#664) each have an issue, so the plan is not the only record of what is outstanding. Also notes #661, the usage-policy fold test that has never passed since it was written, which is the one failure npm test reports throughout this work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * LLP 0193: Draft -> Accepted Follows the corpus pattern set by LLP 0128, the other install-scale RFC: the rfc is Accepted (the proposal was agreed) while execution state lives in the design and plan documents (0135 Active, 0136 Implemented). Here that is 0194, which is Active and becomes Implemented when T8/T11/T13/T2-part-2 land. The contested part, removing disable-model-invocation, was decided by the maintainer on 2026-08-06 and is recorded in #gate-moves-to-the-command. The remaining tasks are scheduled work tracked in 0194 and in issues #660, #662, #663 and #664, not unresolved proposals. Accepted also makes it immutable, which is right for a document whose anchors are now @ref'd from shipped code: changing what it settled needs a new LLP, not an edit that would silently drift those references. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Delete the report skill's asset copies: nothing reads them (LLP 0194 T7) The bundled report skill shipped style.css, copy-md.js, head.html and both favicons in each host tree, 64 KB total. Nothing reads them. They existed because the skill used to copy the stylesheet into the reports tree itself (`cp "$SKILL_DIR/assets/style.css" assets/style.css`). T7 replaced that with the command owning the base sheet, and T10 deleted the copy step from the prose. `hyp report render` installs assets from src/core/reports/assets/, which is its own directory, so the skill never touches them. Verified by wiping a reports tree's assets/ entirely and rendering: all five assets plus theme.css were rebuilt. Every remaining `assets/...` mention in the skill refers to a path inside the reports tree or inside a published report on the server, never to the skill's own copy. report-assets-canonical.test.js loses its reason to exist in its old form: it held three copies byte-identical, and there is now one. It keeps the checks that still mean something (the canonical set is complete, render.js exists, build.sh stays gone) and gains one that replaces the sync assertions: no bundled skill ships a copy of a renderer asset. Verified it bites by restoring style.css to the skill. This is the drift problem solved by construction rather than by policing. Three copies of the same bytes is the shape that drifts, and this exact set did: the canonical stylesheet and the one in a live reports tree diverged, and the skill's "is this an older sheet?" heuristic could not tell a customization from rot. One copy makes the question unanswerable. Package drops from 896 to 892 files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Renumber to LLP 0196/0197: 0193 and 0194 were taken on master while this branch was open master merged llp/0193-openclaw-backfill-cli-denylist.decision.md and llp/0194-per-message-provider.decision.md during this work, so both numbers were claimed twice. LLP numbers are globally unique and never reused, so this branch moves rather than the merged docs. 0195 is taken by PR #667, hence 0196/0197. Renaming an LLP is a mechanical edit (LLP 0156), and it has to be exhaustive or the @refs in shipped code point at someone else's decision. Every rewrite here is anchored to my slug or to an anchor only my documents define, so the openclaw refs (LLP 0193#decision, LLP 0194#decision) are untouched, as are LLP 0161/0172/ 0173 and docs/ACCEPTANCE.md. Updated: both filenames and titles, the forward-refs on LLP 0142 and LLP 0155, all 9 @ref annotations across src/ and test/, prose references in the skills, src/core/reports/README.md, and the CI workflow comment. Verified every renumbered anchor resolves, and test/core/llp-ref-hygiene.test.js (which arrived from master in the same merge) passes. Also merges origin/master, which brings the openclaw backfill work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix stale LLP prose refs missed by the 0193/0194 renumber, doc drift, and add renderer unit tests Four space-form `LLP 0193`/`0194` references (not caught by the @ref scan, which only matches `@ref LLP NNNN#anchor`) still pointed at master's merged 0193/0194 docs instead of this PR's own now-renumbered 0196/0197. A sweep grep confirms no other stale references remain; all other 019[34] hits are legitimate references to master's openclaw-denylist (0193) and per-message-provider (0194) decisions. Also: - rendering.md (claude + codex copies) claimed the renderer enforces the structural contract at runtime and fails if it breaks. render.js's only throw is the pandoc-missing check; the contract is covered by tests over a synthetic fixture, not the user's real output. Corrected the claim and restored the deleted step-6 greps against the user's actual built html/. - codex/src/index.js's activate() JSDoc still listed hypaware-graph, the AI usage report skill, and the report-to-html renderer as contributed skills; none are registered any more. Corrected to the four skills actually registered (hypaware-query, hypaware-reference, hypaware-privacy, hypaware-report). - components.md and example-enrichment.md (both trees) still pointed at `build.sh`, which this PR deletes and report-assets-canonical.test.js asserts must never return. Repointed to `hyp report render`. - reports/README.md cited `git show 0f3dce1^:...build.sh`; that sha does not survive a squash merge. Replaced with a stable description plus a PR reference. - Added unit tests for docLabel, pageTitle, masthead, and discoverSections (previously zero test references), and extended the existing HTML-escaping test to also cover a stat label rather than only the card title. Verified each closes the mutant it was written against (date-slice off-by-one, dropped .trim(), dropped escapeHtml(stat.label)). Left components.md's landing-page per-entry rules alone (pre-existing, deliberate narrowing that needs contributor judgment) and claude/src/index.js untouched (pre-existing inaccuracy, out of scope). Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: test <test@test.com>
|
The residual findings from this PR's review are now tracked in #671, labelled They were handed back here via #671 separates them into three that are mechanically fixable (the two-entry |
Setup proved it had captured something (LLP 0135 #first-look) but never taught what to do with it. Ten skills land in
~/.claude/skillsand the finale reportsskills: 10 copied- a file operation, not a capability. The gap was activation, not information: printing example prompts still leaves the user to open a terminal, start a client, and retype one, and the drop-off across those three steps is the whole distance between "installed" and "used".Both first-party CLI clients take the opening prompt as argv (
claude [options] [prompt],codex [OPTIONS] [PROMPT]), so setup can be the launcher.What it looks like
After the first look and the privacy narration:
Enter, and Claude comes up already working on that question. No copy-paste step.
Design notes
Full rationale in LLP 0195. The decisions worth reviewing:
The manifest owns the launch; core owns the questions.
contributes.client.launch({ bin, args, label }) says how to start a client on a question, so a future adapter becomes launchable with no core edit - the same rule LLP 0180 set for the finale's client list. A spec missing the{prompt}placeholder is rejected at catalog build: launchable-and-mute looks like the feature working.Launchability is a
$PATHprobe, not the picker's detection. A~/.claude/settings.jsondoes not imply aclaudeon$PATH, and Claude Desktop is detectable, pickable, attachable, and has no prompt argument at all. Only clients both picked and resolvable are offered - starting an unattached client would open a session nothing captures.An empty cache suppresses the launch and says why. Every question is about recorded history, so a fresh install with nothing backfilled would spend its one first impression on an empty answer. The first look's result already distinguishes the cases:
slowmeans the opposite of empty and still launches;error/unknown never withholds the offer.Nothing here can fail a finished install. Missing binary, spawn error, cancel, unforeseen throw - all degrade to the printed list. The child's exit code is deliberately not propagated: quitting the client with ctrl+c is not a failed install.
The launched session is not pre-authorized. No
--allowedTools. The client asks its own permission the first time the skill runshyp query, and that prompt stands. A tool whose pitch is "see what your AI clients are doing" should not open by quietly widening what one of them may do.hyp askmakes the list re-runnable, andhyp ask "<question>"skips the menu. Every other closing surface names a durable entry point; a menu reachable only by re-runninghyp initwould be the exception.Two changes to existing behaviour
The prompt is framed (#frame), which no other wizard prompt is. Position is what makes it necessary: every other prompt owns its screen, while this one lands under the first look's tables, rules and dim footers, where an unframed list of sentences reads as more output to skim rather than the one thing still awaiting a keypress. The frame is a shape rather than a colour, so it survives
NO_COLORand a colour-blind reader, and is suppressed rather than soft-wrapped when it would not fit - a broken rectangle is worse than no rectangle.Setup's first look runs
['models', 'daily'], not all four sections (#wizard-sections). This takes up the seam LLP 0135 explicitly left open ("the seam for a shorter variant remains if setup output ever needs trimming") and reverses its judgment that all four was right, recorded as a new decision with a forward-ref rather than an edit to what 0135 settled. Both halves of the cost were real: ~60 lines, and ~5s on a 91k-row cache against a planner that budgeted ~2s - so setup routinely printed "Stopped here to keep setup moving - the repos and tools sections did not finish". A block that regularly announces its own truncation is worse than a shorter block that finishes.hyp query overviewis unchanged and still renders all four.That trim needed one fix in the shared module:
missingSectionsnow takes the requested set. Without it the wizard would claim repos and tools "did not finish" on every run - a claim about work nobody started, and the exact thing that sentence exists to prevent.Known issue, not fixed here
The overview planner's
SECTION_COST_VS_PROBEis calibrated at 1.9x and measures ~3.4x on a real cache, so it picks windows it cannot finish. Filed as #665 with measurements. The trim reduces how often the wizard trips over it but does not fix it, and a real run after this change still hit the deadline - the issue carries that evidence and argues against the obvious fix (re-measuring the constant), since the probe reads a prunable column and structurally cannot predict what the sections cost.Testing
npm test: 3613 pass, 1 fail -usage-policy-fold.test.js:387, confirmed failing identically atmasterin a clean worktree. Unrelated to this branch.npm run typecheck: clean.walkthrough_picker_to_first_query,cli_bundled_plugins_activated,package_bin_boot,status_diagnostics.test/core/cli/wizard/first-ask.test.js(16),test/core/client-launch-manifest.test.js(4), plus the framed-render and wizard-orchestration cases. Launch tests stub the spawn seam - no test starts a real client.claudeandcodexresolve from the real manifests;hyp ask,hyp ask --list, and the no-launcher path (empty$PATH) behave as described.🤖 Generated with Claude Code