Uh oh!
There was an error while loading. Please reload this page.
fix(cli): reject unknown arguments on every visible core command - #850
fix(cli): reject unknown arguments on every visible core command#850philcunliffe wants to merge 6 commits into
Conversation
Core commands did not share one argument-validation contract. Half the set parsed argv through parseCommandArgv() and exited 2 on a token it did not know; the other half hand-read the two flags it cared about and dropped the rest, so `hyp daemon status --jsn` printed the human table and exited 0 while `hyp sink maintain --jsn` exited 2. A script cannot tell those apart, and around `daemon stop`, `daemon restart`, and `report delete` the quiet reading is the dangerous one. Sixteen commands were affected: version, status, ask, backfill list, daemon status/stop/restart, plugin list/info/outdated/remove, query schema/status/refresh, remote add/list/remove/login, and the report subcommands (whose positionals()/valueFlag() helpers skip flags they do not recognise rather than refusing them). They now parse through parseCoreCommandArgv(), a thin wrapper over the existing codec that writes the refusal in one place: exit 2, the token named on stderr, nothing on stdout. Each command's schema and its registered usage line are one CORE_COMMAND_ARGS entry, so core_commands.js reads the usage string from the same declaration the parser enforces rather than holding a second copy. LLP 0266 records the contract and LLP 0009 carries the forward-ref. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…#853) `test/core/repo-scratch-hygiene.test.js` has been failing on `master` since it merged: `x/npm-test.log` and `x/typecheck.log` are tracked, and the test's first half asserts no `.log` is. Both came in on `adb448ab` (#785) via the `git add -A` sweep that #786 wrote this test to catch; the files predate the test, so it was red on arrival. Every branch cut since inherits it, currently blocking #833, #849, #850 and #851 for a reason none of them caused. The transcripts are deleted rather than the test relaxed, which is what its message asks for. `.gitignore` needs nothing: `*.log` is already committed and the rule test already passes, since an ignore rule cannot reach a path that is already tracked. That asymmetry is the whole reason the file carries two tests. Scope is exactly the two `.log` paths. The other eight files under `x/` are untouched: the hygiene test forbids tracked transcripts and nothing else, and sweeping up scratch it does not name would be a judgement this fix has no authority to make. Fixes#852 Co-authored-by: test <test@test.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
`hyp query schema` is not a verb and has never rendered anything but its plain-text listing, but three shipped agent-facing files told their reader to run `hyp query schema <table> --format json` (and one `--format markdown`). Before this branch the flag was silently dropped and the listing printed; with the strict gate the same invocation exits 2 with an empty stdout, so the skills' prescribed first step for an unfamiliar table now fails. Drop the flag from both hosts' `hypaware-query` skills and from the `hypaware-analyst` agent, and record the consequence in LLP 0266. The two skill trees are edited identically, so the host-divergence record is unchanged. Also re-attach the `@ref LLP 0266#usage-agreement` annotation: a blank line sat between it and `CORE_COMMAND_ARGS`, which breaks attachment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
philcunliffe
commented
Aug 19, 2026
Review: |
…hat the body reads - An unaliased single-dash token was still a positional value, so `hyp query refresh -f` looked up the dataset '-f' (exit 1) and `hyp report get k p id -o out.html` fetched the artifact 'out.html' instead of writing the file. `parseCoreCommandArgv` now opts into `strictShortFlags`; the verb family keeps the lenient reading for greedy positionals that legitimately carry '-1'. - `report list` and `report delete` gated on the schema and then re-read argv, so `--json=true` / `--yes=true` validated and were then dropped. Both read the gate's params now. - `remote login`'s help path returned reason 'ok', which tells a LoginOutcome reader the sign-in succeeded. It gets its own 'help'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0266 is also claimed by fix/issue-836 (PR #850) and update/icebird-squirreling-native-batches (PR #866); 0267 is also claimed by fix/issue-837 (PR #849). Both of those PRs are older, so this branch yields the numbers. 0276 and 0277 are free across master and every open branch. Mechanical renumber only: no content change (LLP 0156). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review round on |
philcunliffe
commented
Aug 19, 2026
Triage at head 8607105: the review-round cap was reached, so the residual findings were classified. Both review rounds' findings (rounds on 151ea60 and 412c5a6) are verified fixed at this head; the regression test file passes 117/117 here. The one remaining item, unknown short flags still binding as positionals on the 12 already-strict commands outside CORE_COMMAND_ARGS, is pre-existing on master and deliberately recorded in LLP 0266 #open-questions item 2, so it is non-blocking. Deferred to #903. This PR can merge safely. |
…onfig is not a reconfigure (#874) * LLP 0266: a hidden picker row stays off the sync gate too On every enrolled machine the sync gate led with the two hidden raw-proxy rows (raw-anthropic / raw-openai) wearing the fleet label, because @hypaware/ai-gateway sits in the central layer so they classify locked - while the picker had deliberately never offered them. The lane's locked descriptors now go through the same visiblePickerDescriptors filter the pick lane uses, at the screen, never at the locked set (which would re-compose the org gateway into the local layer, LLP 0129). The no-candidates short-circuit splits accordingly: with no visible org row to name, it says nothing syncs instead of naming the fleet as owner of an empty list. Extends LLP 0202 and supersedes its "sync/opt-out menu is unchanged" consequence line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * LLP 0267: an answer-less config does not make a reconfigure hyp remote add before the first hyp init writes a config holding only query.remotes. The pick phase classified any readable config file as a reconfigure, so that run seeded from an empty read-back instead of from detection: every box arrived unchecked, no defaults gate rendered, and the export default quietly flipped to keep-local. The classification now keys on whether the config records a pick answer, discriminated by the plugins key: the composer always writes a plugins array, the side-channel writers never do. An answer-less config seeds like no config at all - detection pre-checks, gates render, export takes the first-run local-parquet default - while its own keys still carry through the composition fold. plugins: [] stays a reconfigure: an emptied install must not be re-consented from detection. Extends LLP 0183. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Review round: hidden rows off the sync gate's candidate list too, and the no-candidates line stops claiming nothing syncs Two defects in the LLP 0266 half of this PR, both on the sync gate. The locked list went through `visiblePickerDescriptors`; the candidate list did not. A carried hidden row (LLP 0202 #carry-through) reaches `picked.descriptors` whenever that row is not locked - a team join whose org config has not converged, or a machine whose central layer does not declare `@hypaware/ai-gateway` - and the gate then rendered it as an editable checkbox for a row the picker deliberately never offered, where unchecking it writes a `local-only` entry for a source the user never saw. Both row lists now take the same filter, which is what makes LLP 0266's "absent from every wizard screen" true rather than half true. The new no-candidates line then said "nothing syncs to your server" on exactly the machine class LLP 0266 targets: an enrolled machine whose locked set is entirely the hidden `raw-*` pair filtered out of the display. Those rows are still locked, still composed by the org's central layer, and under LLP 0188 #locked they always sync and can never be opted out - so the sentence traded LLP 0202's over-disclosure for an affirmatively false claim about what leaves the machine. The branch now splits three ways on `lockedHidden`, a count the lane gets so it can tell the truth about withheld rows without being able to name them. LLP 0266 §sync-gate, §no-candidates, and §consequences updated to match. * Renumber LLP 0266/0267 to 0276/0277 to clear number collisions 0266 is also claimed by fix/issue-836 (PR #850) and update/icebird-squirreling-native-batches (PR #866); 0267 is also claimed by fix/issue-837 (PR #849). Both of those PRs are older, so this branch yields the numbers. 0276 and 0277 are free across master and every open branch. Mechanical renumber only: no content change (LLP 0156). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Sync gate must not claim nothing syncs while a hidden picked row stands The LLP 0276 filter takes hidden rows off both sync-lane lists, but only the locked list reported how many it removed. A carried hidden row (LLP 0202 #carry-through) that is not locked reaches picked.descriptors, is composed into the local layer, and syncs unless an opt-out entry says otherwise - and when it is the only pick, the filter empties `candidates` with `locked` empty and `lockedHidden` 0, so the lane took the strongest of its no-question sentences and told the user "nothing syncs to your server" while capture was in fact leaving the machine. That is the affirmatively false claim LLP 0276 #no-candidates ruled out for the locked case and did not carry to the candidate case. `runInitWizard` now passes `candidatesHidden` alongside `lockedHidden`, one count per filtered list, and the no-question branch gains a fourth line: with no locked row but a hidden pick standing, it states that capture already set up on this machine still syncs, naming neither the row nor the fleet (which does not own it). LLP 0276 #no-candidates extended with the case and the sentence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: neutral <neutral@hyperparam.app> Co-authored-by: test <test@example.com>
# Conflicts: # src/core/commands/query.js
# Conflicts: # src/core/commands/status.js
philcunliffe
commented
Aug 19, 2026
Both PRs are mergeable and green right now, and merging both will conflict. Re-derived from git, not inferred:
So #914 is this PR's tree plus one more commit. That is expected: #903 is the deferred-findings follow-up to this PR (#850), and its branch carried this work forward rather than branching from a merged base. What this means for the merge button: merging #914 alone delivers everything here. Merging this one too, in either order, hands you a conflict on the second. neutral is not choosing between them, and has changed nothing on either branch beyond the base merges already reported. Filed by neutral during reconcile. Reply here if you want it to act on this rather than just report it. |
philcunliffe
commented
Aug 19, 2026
Triage at head 82ca71a: the review-round cap was reached, so the residual findings were classified against the current tree. All six findings from the two review rounds (on 151ea60 and 412c5a6) are verified still fixed at this head, which differs from the last-triaged 8607105 only by two merges of origin/master:
Verification at this head: The one remaining item, unknown short flags still binding as positionals on the 12 already-strict commands outside This PR can merge safely. Note the standing merge-coordination report above: #914 fully contains this tree plus one commit, so a human should merge one of the two, not both. |
Problem
Core commands did not share one argument-validation contract. Half the set parsed argv through
parseCommandArgv()and exited 2 on a token it did not know; the other half hand-read the two flags it cared about and dropped the rest.Measured on
masterat a14246d by dispatching every visible core command with--definitely-not-a-real-flag, sixteen accepted it:version,query status,daemon stop,daemon restart--jsonstatus,daemon status,backfill list,plugin list,plugin outdated,remote listplugin info,plugin remove,query schema,query refreshaskpositionals()/valueFlag()remote add,remote remove,remote login,report render/publish/list/get/deleteSo
hyp daemon status --jsnprinted the human table and exited 0 whilehyp sink maintain --jsnexited 2. A script cannot tell those apart, and arounddaemon stop,daemon restart, andreport deletea dropped flag is a different operation, silently.Fix
src/core/cli/command_args.js:CORE_COMMAND_ARGS, one entry per migrated command holding both its parser schema and its registered usage line, plusparseCoreCommandArgv(), a thin wrapper over the existingparseCommandArgv()codec that writes the refusal in one place (exit 2, the token named on stderr, nothing on stdout; a non-leading--helpprints usage on stdout and exits 0).daemon startanddaemon uninstallmoved off their hand-rolled loops onto the same helper.core_commands.jsreads each covered command'susagefromcoreUsage(name)instead of holding a second copy, so the advertised line and the enforced schema are one declaration.hyp remote loginstill says "--org expects an org name" (covered by existing tests), and the gate adds the refusal for everything no reader names at all.hyp remote login's usage line becomes[name]rather than<name>, matching what it has done since LLP 0062 #bare-remote.Regression test
test/core/cli-arg-validation.test.js(91 tests, all new):hyp <cmd> --definitely-not-a-real-flagand assert exit 2, the token named on stderr, and an empty stdout. Onmaster16 of these fail (hyp remote list ... exited 0,hyp plugin info ... exited 1, and so on) andhyp remote loginhangs waiting on stdin; on this branch all 65 pass.CORE_COMMAND_ARGSentry: every--flagin the usage line is a non-positional schema property and every non-positional property appears in the usage line, plus every positional binds to a real property.Design
llp/0266-core-command-argument-validation.decision.mdrecords the contract; LLP 0009 carries theExtended-by:forward-ref. The doc is explicit that this is reuse of the argv codec, not the verb registration LLP 0034 #verbs bounded: these stayctx.commands.register, contribute no MCP tool, and gain norender. It also records what was deliberately not done (generating usage lines from schemas, which cannot expresshyp purge <path> | --session <id> | --ignored | --all) and that already-strict commands keep their schemas next to the code that reads their params.Verification
npm test: 4287 pass / 22 fail. The 22 are identical toorigin/masterin this sandbox (missing optional deps:marked,@aws-sdk/*, parquet/iceberg backends). Baseline was 4196 pass / the same 22 fail, so this adds 91 passing tests and breaks nothing.npx tsc -p tsconfig.json --noEmit: 7 errors, byte-identical to theorigin/masterbaseline, allCannot find modulefor deps absent here. None in changed files. (npm run typecheckitself cannot run in this sandbox:@types/nodeis not installed, on master too.)bin/hypaware.jsconfirming happy paths are unchanged (version,status --json,daemon status --json,daemon stop,query status,remote list --json,plugin list/outdated --json,backfill list --json,ask --list,query schema logs,remote add/remove) and refusals are uniform.Acceptance criteria
CORE_COMMAND_ARGSrecords the migrated surfaces, the LLP the rest).parseCommandArgv().-yand-f(untouched; the spec carries analiasesfield and the commands that use them,plugin install/update,daemon run, keep their own).hyp mcpis untouched.One acceptance item is deliberately narrower than written:
CORE_COMMAND_ARGScovers the commands migrated here, not the whole core set. Folding in commands that already parse strictly (init,sync,purge,policy *,attach/detach,plugin install) would mean moving their schemas away from the code that consumes their params, which is the drift this is against. They are held to the contract by the parameterized test instead. LLP 0266 #open-questions records this.Fixes#836