Uh oh!
There was an error while loading. Please reload this page.
hyp policy answers in the vocabulary it teaches - #411
Conversation
`hyp policy set <p> sync` confirmed with `marked <p> as full (.../usage-policy/local-only.json)`: the internal class and the backing store file, leaked into the one line whose whole job is to confirm the word the user typed. It reads as though `sync` had become `local-only`, the exact inversion LLP 0110 minted this verb to kill. The `policy` runners now pass a `PolicyHumanVocabulary` into the shared marking / unmarking / check writers, so `set`, `show`, `unset`, and `list` answer in the sync|local-only|ignore token vocabulary and name the machine-local policy store instead of pathing it. A governing `.hypignore` is still named by its real path: it is a file the user can edit, not an internal. The writers' default vocabulary is the verbatim-internals one, so the deprecated `hyp ignore` / `hyp unignore` flag aliases keep byte-identical output by construction (LLP 0111 #aliases), now pinned by exact-output tests. The on-disk `full` class, the `local-only.json` store, and every `--json` shape are untouched. Co-Authored-By: Claude <noreply@anthropic.com>
…a corrupt store Review fixes for #411 (issue #393): - policy show now suffixes the class label with "(implicit default, not yet classified)" when nothing governs the directory, gated through a new optional PolicyHumanVocabulary#implicitSuffix member that defaults to a no-op so the deprecated `--check` alias keeps its byte-identical bare `class: full` output. Without this, an unmarked directory rendered identically to an explicit `sync` mark, which the hypaware-privacy skill could misread as a recorded consent answer. - policy show / policy list catch LocalOnlyListUnreadableError at the policy edge and report "the machine-local policy store at '<path>' is unreadable or malformed" instead of leaking the resolver's "local-only list" wording; hyp status and the flag aliases are untouched. - policy set now suffixes its confirmation with "(machine-local policy store)", matching the no-op and policy list lines that already said the marking never leaves the machine. - LLP 0111 #tokens/#show updated to document both behaviors. --json output, the on-disk store format, and all three deprecated alias forms remain byte-identical.
philcunliffe
commented
Jul 27, 2026
Review round 1 - |
runPolicyShow and runPolicyList already caught LocalOnlyListUnreadableError and reported it with the neutral "machine-local policy store" wording; runPolicySet and runPolicyUnset called the shared writers bare, so the error fell through to the generic dispatcher and printed the writers' internal "local-only list" wording plus the store path unprefixed - the exact leak issue #393 minted this verb to stop. Wrap all four `policy` runners uniformly, add corrupt-store tests for set/unset mirroring the existing show/list pair, and correct two doc overstatements: the runners-only scope of reportUnreadableStore now names all four runners, and LLP 0111's claim that no policy line ever names the store path is corrected since the corrupt-store message and `policy list`'s trailing parenthetical both do, deliberately.
philcunliffe
commented
Jul 27, 2026
Review round 2 - |
| Constraint | Result |
|---|---|
policy show --json byte-identical | PASS, 0 hunks across unmarked, full, local-only, ignore, .hypignore-governed, cwd-default |
policy list --json byte-identical | PASS, 0 hunks across empty / one / two entries |
| On-disk store unchanged | PASS, byte-identical including multi-entry and every class |
hyp ignore --sync / --private / --local-only | PASS, 0 hunks |
hyp ignore --check on a marked dir | PASS, class: full / governed-by: <listPath> |
hyp ignore --check on an unmarked dir, no implicit suffix | PASS - the exact path fix 1 touched |
hyp unignore --sync | PASS, 0 hunks |
hyp status on a corrupt store | PASS, still local-only exclusion list at '…' |
| Aliases on a corrupt store | PASS, unchanged |
The only behavioral diffs against master are the 17 intended ones.
"Correct by omission" holds structurally, not merely by test: the aliases never construct or pass a PolicyHumanVocabulary, so the parameter default INTERNAL_VOCABULARY applies, and the new optional implicitSuffix is absent by design and null-coalesced at its single call site. One latent hazard worth knowing: a future optional member used without a ?? noop at its call site would TypeError on the alias path rather than fall back. Worth a comment on the interface if more optional members land.
Fix-by-fix
- Fix 1 (implicit default) is correctly gated.
!result.governedByis exactly "nothing governs this path":governedBy: nullis produced at one place only (usage-policy/matcher.js:144), and an ancestor machine-local entry or a.hypignoreboth set it, so neither gets the suffix. - No machine parser exists for the
class:line, checked before accepting the suffix: bothhypaware-privacySKILL.md copies referencehyp policy showin prose only,docs/PRIVACY.mdis prose, and no smoke flow, hook, or script reads it. The only^class: …$regexes in the tree are in tests. A^class: (\w+)$parser would have broken, but none exists. - Fix 2's blast radius is safe:
err.filePathis always set at every construction site, the message is built from it rather than a re-derived path, exit code stays 1, and the change only adds catches (nothing previously surfacing is now swallowed). - Test quality: each of the four fixes has a test that fails if reverted, including a negative alias guard asserting
doesNotMatch(/implicit default/). The alias-stability tests use exact-string rather than loose-regex assertions.
Verification run
node --test on the two policy test files: 52 pass, 0 fail (35 + 17). npm test: 2680 tests, 2671 pass, 8 fail, all in test/core/leave-command.test.js, the only failing file, count unchanged from master's pre-existing 8. npm run typecheck and npm run build:types both clean. After the round-2 fix: 54 tests in the two files, all passing, and the four policy subcommands now print the neutral wording uniformly while the three alias forms and hyp status are byte-identical to before.
The head has moved to a541d62, so the next tick reviews that head. Held for a human: neutral does not merge.
philcunliffe
commented
Jul 28, 2026
Triage: shippableThe review loop hit its round cap with this head unreviewed, so the remaining question was judged rather than re-reviewed: can this ship safely? It can. All five findings from the two rounds were re-derived from the tree rather than taken on trust, and each fix is really there and really works. The never-reviewed head ( The three hard constraints were re-verified live against a from-scratch checkout of the pre-PR base run side by side, not by re-reading earlier claims:
Interaction hunting found nothing: governor naming stays correct across both branches of One item was deferred to #413: the corrupt-store catch no longer reaches the dispatcher's generic catch, so it loses that catch's
Held for a human. neutral does not merge. |
Uh oh!
There was an error while loading. Please reload this page.
Before
After
A governing
.hypignoreis still named by its real path inpolicy show:that is a file the user can open and edit, not an internal.
Root cause
src/core/commands/policy.jsmaps the public tokensynconto the storedclass
fullat the parser edge (TOKEN_TO_CLASS), and then hands the storedclass to the shared writers, which print internals verbatim:
src/core/commands/clients.js:905(pre-fix) wrote the confirmation asthe template
marked <resolvedTarget> as <targetClass> (<listPath>), wheretargetClassis the storedfullandlistPathis the backingusage-policy/local-only.json.src/core/commands/clients.js:890(pre-fix):already ${existing.class} (governed by ${existing.governedBy}).src/core/commands/clients.js:1089,1091(pre-fix,runIgnoreCheck, sharedwith
policy show):class: ${result.class}andgoverned-by: ${result.governedBy}.src/core/commands/clients.js:1022,1026(pre-fix,runUnmarkMachineLocal):the removal and no-op lines print
targetClass/entry.class.src/core/commands/policy.jsrunPolicyListprintedfull (sync)and thebare store path.
The mapping was one-way: public token in, internal class out. There was no
translation back at the human-output boundary.
Fix
The four
policyrunners now pass aPolicyHumanVocabulary(new interface insrc/core/commands/types.d.ts) into the shared writers. It renders a storedclass as its CLI token, names the machine-local store instead of pathing it,
and drops the trailing store path from the
setconfirmation. The writers'default is
INTERNAL_VOCABULARY(internals verbatim), so the deprecatedhyp ignore/hyp unignoreflag aliases keep their exact legacy output byconstruction rather than by test discipline (LLP 0111 #aliases). LLP 0111
#tokens / #show / #list / #aliases are updated in this commit to describe the
bidirectional mapping and to record the alias divergence as deliberate.
Regression test
test/core/policy-command.test.js(5 new tests, plus 6 existing pins that hadencoded the buggy wording). Run on the pre-fix tree,
node --test test/core/policy-command.test.jsgives# pass 22 / # fail 9, for example:After the fix:
# tests 31 / # pass 31 / # fail 0.The
--jsoncontract and the on-disk format are untouchedEvidence, all in-tree and all green:
hyp policy show --json keeps the stored vocabulary and the store path (unchanged machine contract)(new): asserts the stdout ofpolicy show --jsonis byte-equal toignore --check --json, and that thepayload still carries
class: "full"andgovernedBy: <.../local-only.json>for a storedfullentry.policy show [path] --json is byte-compatible with hyp ignore --check --jsonand bothpolicy list --jsontests are unchanged andstill pass;
runPolicyList's--jsonbranch returns before any humanrendering.
policy settest still asserts the store contents viareadLocalOnlyEntries, e.g.set <p> syncstill writes[{ dir, class: 'full' }]. No store read/write helper, no store version,and no class lattice code was touched.
test/core/ignore-private-sync-command.test.jspin the deprecated aliases:hyp ignore --syncstill printsmarked <p> as full (<listPath>)andhyp ignore --checkstill printsclass: fullwith the real store path.They pass both before and after this change.
Suite status
npm test: 2675 tests, 2666 pass, 8 fail, 1 skipped. The 8 failures are thepre-existing
test/core/leave-command.test.jsones (verified by stashing thisbranch's changes and re-running that file on the base commit: 11 tests, 3
pass, 8 fail, identical set).
npm run typecheckis clean.npm run smoke -- local_only_export_withholdpasses.Fixes#393