Uh oh!
There was an error while loading. Please reload this page.
report commands act on the --remote and --output the gate validated, not the first argv match - #949
Closed
philcunliffe wants to merge 10 commits into
Closed
report commands act on the --remote and --output the gate validated, not the first argv match#949philcunliffe wants to merge 10 commits into
philcunliffe wants to merge 10 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>
`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>
…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>
…ec directly LLP 0266 D1 says an unaliased single-dash token is a flag on every visible core command, but the mechanism only reached the 25 commands in CORE_COMMAND_ARGS. The 13 that call parseCommandArgv() directly and bind a positional still read '-Z' as that positional: 'hyp policy show -Z' reported on a directory named '-Z' and exited 0, 'hyp sink maintain -Z' looked up a sink named '-Z' and exited 1. verb_codec.js exports STRICT_SHORT_FLAGS, and each such call site opts in. The per-site look for a legitimate dash-leading value found one, 'hyp join's opaque token positional, so join stays lenient; 'query sql' is a verb and keeps the lenient reading D1 carved out for a greedy SQL positional. The parameterized short-flag test now iterates the visible command set rather than the table, so a new call site cannot re-open the gap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # src/core/commands/query.js
…y test runs the command Two silent drops in `report_commands.js` survived the strict gate: it validated `--title` and `--limit`, then read them back with `valueFlag()`, which returns undefined for any value whose first character is '-'. So `hyp report publish --title '-Q3 rollup'` published untitled and `hyp report list --limit -5` listed with the server default, both exit 0 with nothing on stderr. Both now read `gate.params`, the way `--json` and `--yes` already do here. The new "the verb family stays lenient" test called `argvToParams()` without `strictShortFlags`, so it only re-proved that the lenient reading is lenient and would still pass if `query sql`'s call site started opting in - the exact regression `SHORT_FLAG_LENIENT` takes the command out of the parameterized case to allow. It now runs `hyp query sql select -1 as n` through dispatch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… required positional refuses Three findings from the review of 342ff09, all the same shape the PR was already closing: a token the argument gate validated, then dropped or re-read out of raw argv by the code that consumes it. - `remote login` read `--browser`, `--no-browser`, `--no-forward` and `--no-daemon` with `argv.includes()`. The codec also accepts the `--flag=true` form, so `hyp remote login prod --no-forward=true` was blessed by the gate and then ignored: the machine enrolled for fleet forwarding against an explicit opt-out (LLP 0063 D3), exit 0, nothing on stderr. The four flags and the target name now read `gate.params`. - `remote add`, `remote remove` and `query schema` replaced hand-written `if (!name)` guards with the schema's `required`, which tests only `!== undefined`. An empty argument stopped being refused: `hyp remote add "" https://x` exited 0 and wrote a remote named '', where master exited 2. On a command line an empty token is an unset shell variable, so the argv path now treats it as missing. The MCP path keeps the plain test, where a JSON `""` is explicit. - `applyOrgParam()` was the last argv reader in `report_commands.js`. `valueFlag()` drops a dash-leading value and reads the first occurrence while the codec validates the last, so `--org -acme` sent no org at all and `--org a --org b` validated 'b' and sent 'a', across publish, list, get and delete. LLP 0266's Context said "sixteen visible core commands" where the list below it enumerates eighteen plus the `report` subcommands; corrected, with the repeat in Consequences. Editorial only: nothing the decision settled changes. Six behavioural regression tests, each failing on the pre-fix tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t the first argv match (#941) The two raw-argv readers deferred out of PR #914 as "preference" are the same defect class the review already closed for --org: `valueFlag()` takes the FIRST occurrence of a flag while the codec keeps the LAST, so the gate validated one token and the command acted on another. - `resolveReportsTarget()` read `--remote` from argv. `hyp report list --remote a --remote b` validated 'b' and sent the request, with b's credential resolved, to a. The target selects the server, so on `report delete` that is a destructive call against a scope the gate never blessed. - `runReportGet()` read `--output` from argv. `--output a --output b` validated b and wrote the bytes to a, exit 0, with `saved N bytes to a` on stderr. Both now read `gate.params`, which retires the last `valueFlag()` reads in `report_commands.js`. The `present && !value` guards go with them: the gate already refuses a flag with no value ahead of both call sites, and a dash-leading target now reaches the registry lookup, so the refusal names the token instead of a generic "expects a target name". Three regression tests in test/core/report-commands.test.js, all verified failing on the pre-fix tree (not ok 21, 22, 26) and passing after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
philcunliffeforce-pushed
the
fix/issue-903
branch
from
August 19, 2026 20:02
eb9b97e to
627d251Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes item 3 of #941 (the two remaining raw-argv readers in
report_commands.js). While writing the regression tests the item turned out not to be preference-only: both readers carry the silent half of the defect class the #914 review closed for--org.The defect
valueFlag()returns the first occurrence of a flag (src/core/cli/remote_commands.js:548-559); the codec keeps the last (src/core/cli/verb_codec.js:198). Thepresent && !valueguard the review credited these two sites with covers the dash-leading case only, not the repeat.hyp report list --remote prod --remote backupbackupprod, exit 0hyp report get k p id --output a --output bba, exit 0,saved N bytes to a--remoteselects the server and the credential resolved for it, so onhyp report deletethis is a destructive call against a scope the gate never blessed. Same shape as the--org a --org bfinding the second review round fixed.The fix
resolveReportsTarget()takes the gate's params instead of argv and readsparams.remote;runReportGet()readsgate.params.output. That retires the lastvalueFlag()reads inreport_commands.js, which was the stated point of the deferred item.The two
present && !valueguards go with them, and nothing is lost: the gate refuses--remote/--outputwith no value (--<flag> expects a value+ usage, exit 2) before either call site runs. The one wording change is that a dash-leading target now reaches the registry lookup, sohyp report list --remote -stagingrefuses withunknown remote target '-staging'rather than a generic--remote expects a target namethat never said which token was rejected.Regression tests
test/core/report-commands.test.js, three cases, all verified failing on the pre-fix tree and passing after:Whole suite on this head:
npm test4696 pass / 0 fail / 1 skipped (up from 4693, +3 new),npm run typecheckclean. CI is authoritative.Base and the other three items
This branch is stacked on
fix/issue-903(PR #914), which is unmerged. The gate this change reads (parseCoreCommandArgvreturningparams) does not exist onmaster, so there is no way to branch this offmaster. Merge #914 first; this PR's own diff is the single commit90865c67.The remaining items in #941 are correctly left alone and no code here touches them:
hyp askwith an unquoted dash-leading word exits 2 - settled by LLP 0266's Consequences. Changing it would be a new LLP, not a fix.So #941 can close when this merges.
Fixes#941