feat: private is not a suggestion (v1.54.0) - #183
Merged
Merged
Conversation
`.apb/runs/` and `.apb/tmp/` are server-local scratch written by the agentic-playbooks runner. They are never committed, but they sit inside the repository tree, so `oxfmt` swept two generated `hooks.json` files and the pre-commit gate failed on drift that belongs to no change. Ignored in the same shape and for the same stated reason as `.worktrees/`. `.apb/config.yaml`, `.apb/profiles/` and `.apb/playbooks/` stay visible on purpose: whether that configuration belongs in the repository is a separate decision, and ignoring it here would settle that question silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 0 and 1 for the visibility-enforcement wave: design, plan, and the consultant audit trail. Both source cards had premises refuted against live source before the brainstorm, and the verdicts are recorded in the design rather than built around. t_92c26f69's "no visibility frontmatter handling in the read surfaces" is refuted by graph/visibility.ts and result-filters.ts, and its proposed carrier, gatedOwnerScopeView, is refuted as an approach: it is inert unless integrity.owner_scope_delivery is fail, so riding it would ship privacy that is off by default. t_8f670d8e's "no registry-driven harness, no fail-closed gate" is refuted by agent-scope-matrix.test.ts and visibility-surface-census.test.ts; what survives is the visibility axis, the _meta channel, the federated caller shape, and the un-swept OpenClaw page walker. The wave's own stated risk is corrected too: indexRevision is a query-cache generation counter, not a schema revision, and bumping it forces no reindex. The real index-side obligations are the seen-before-read ordering in the indexer and the mtime fastpath over pages tagged before the boundary shipped. The consultant's recommended variant is adopted with one mechanism replaced. Reading a missing index column as private would blank out search in every vault on upgrade, including the vaults that never used the field; migration 12 backfills instead, from the frontmatter the index already stores at chunk_index 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`visibility:` shipped as a caller-liftable scope: a remote caller that asked for the private scope was given it, so the tag was decorative. This is the predicate that stops being decorative - the kernel the rest of the wave's three read roots consume. `REMOTE_DENY_VISIBILITY_TOKEN` reserves exactly one token, already in the normal form `pageVisibility` reads tags into, so the rule has one spelling. `isRemotelyReadable` denies it at remote reach and answers `true` for everything else at every reach, leaving all other tokens to `isVisible` unchanged - a vault that never wrote the token sees no change. Two departures from `plan.md`, both named rather than absorbed: - The closed vocabulary lands HERE rather than in the next unit, because the predicate cannot take a reach argument the type for which does not exist yet. The next unit keeps the transport minting and the caller-side refusal. - The design called the vocabulary `disclosure`. That word is already taken in this tree: `SearchOptions.disclosure` is the progressive result-DEPTH mode (`full` | `cards`), and this value has to ride on the same options bag. Two fields called `disclosure` meaning different things is the "one rule, two spellings" defect this suite exists to remove, so the transport fact is named `TransportReach` for what it measures - where the caller reached us from - and what each member actually proves is written down at the definition instead of implied.
…aller The predicate the previous commit added takes a reach. This is where that value comes from, and the answer is deliberately not "the request": a trust claim echoed back from the caller is not a trust claim, the rule `owner-scope-refusal.ts` already states once for owner identity. Each transport mints its own and cannot be talked out of it - the value is set AFTER the caller's runtime options are spread, the same guard `sendNotification` already uses. stdio and the CLI tool-call bridge mint `local`, because the caller already started the process and holds whatever filesystem access it runs with; an HTTP bind mints from `isLoopbackHost` alone, keyed on the BIND rather than a caller-supplied `Host` header. Both handles now report what they minted, alongside the other transport facts they already publish. `assertNoCallerSuppliedReach` runs on the one seam both the JSON-RPC path and the CLI bridge share, ahead of the unknown-argument gate, so a caller naming the boundary is told about the boundary instead of being offered a spelling suggestion for it - and unlike that gate, this rule does not read the schema, so an open one is covered too. Names normalise case and separators, so `reach`, `transport_reach` and `transport-reach` are one argument rather than three holes; `disclosure` is deliberately left alone, being the unrelated result-depth mode on the recall surfaces. Departure from `plan.md`, named rather than absorbed: `ServerContext.reach` is optional on the type and mandatory in effect, read only through `contextReach`. A required field would have meant editing some fifty-five hand-built contexts across the suite for no gain in safety - the property the design wanted is that an unestablished reach denies, and that is what the reader does, in the same fail-closed shape `entityStatusInScope` uses for a status outside its vocabulary.
Root A of three. `applyVisibilityScope` is reimplemented on `isRemotelyReadable` rather than left beside it, so "may this caller see this page" has one spelling in the ranked pipeline instead of two. The caller's `visibility` argument keeps working and keeps narrowing; what it can no longer do is name the reserved token and be handed the page. The reach rides on `SearchOptions.transportReach`, absent resolving to the narrowest, and every caller of `search()` now states which one it is: the MCP surfaces take it from the context their transport minted, the CLI verbs take the one named once in `src/cli/transport-reach.ts`, and the measurement lanes - bench, benchmark, rerank fit - pass local explicitly, because a lane that stopped seeing reserved pages would report a smaller vault than the one it is measuring. The cross-vault union needed no edit: it spreads the caller's options onto every federated leg, so a federated caller is already bound by the same rule. Two defects found in code this change touches, fixed here and named: - `captureRecallFeedback` re-ran the judged query unscoped and returned `resultFound`, which is an existence oracle for any page the caller cannot read. It now takes the caller's reach like every other search-backed surface, and its re-run depth is a named constant instead of an inline 50. - `applyVisibilityScope` swallowed the frontmatter read error and treated an unreadable page as untagged, so a document still in the index whose file was deleted or made unreadable since the last run was served with no visibility claim at all. It now reads the verdict `readCachedFrontmatterEntry` already carries and fails CLOSED at remote reach, the shape `isPathOwnerVisible` next to it already used. The substitution answers the reach question only - letting it reach `isVisible` would have dropped every unreadable page from every default-scope search at every reach, which is a caller-scope rule this boundary has no business changing. `tests/core/search/agent-scope-surfaces.test.ts`'s deleted-file case is updated rather than deleted: its subject is that an unscoped call applies no OWNERSHIP filter, which is still true, and it now says at which reach it is asking. The remote counterpart is pinned beside the other visibility cases.
Root B of three. `listVaultPages` already parses every page's frontmatter, so the decision costs no extra read - the shape `vaultPageInStatusScope` established for entity status. The drop happens before the sort and before the array leaves the function, so no caller can count what it cannot see; a total that still included the withheld page would answer "there is one more page here than I am showing you". The reach is a REQUIRED option rather than a defaulted one, because the two ways to get it wrong fail in opposite directions: a maintenance lane that quietly stopped seeing reserved pages would corrupt the link graph it repairs, while a read surface that quietly saw them would be the boundary failing. Every one of the twenty-three call sites now says which caller it is walking for, and the compiler is what asks the question. `MAINTENANCE_LANE_REACH` names the answer the repair, portability, freshness, schema-report, heal and doctor lanes give, once, so a reader can find all of them by finding the constant. `src/openclaw/index.ts`'s page search - the surface `visibility-surface-census.test.ts` named as un-swept - takes the narrowest reach. The distinction from the CLI is the caller rather than the machine: a CLI verb is typed by the operator, while these tools are called by a model over a conversation this plugin cannot see, so running on the operator's own host proves the process has filesystem access and not that the party asking is entitled to a reserved page. Two defects found in code this change touches, fixed here and named: - `ListVaultPagesOptions` was also the options type of the two cheap basename walkers, which never parse frontmatter and therefore cannot answer the visibility question. A shared type would have let them claim an answer they cannot give, so the walk options are now a base `VaultWalkOptions` and only the frontmatter-parsing walker carries the reach. - `second_brain_query`'s `total_pages` counted the pre-filter walk, so the status-scope filter it already applied was observable as a difference between `total_pages` and `returned`. Filtering at the root closes that for both rules at once. `openclaw/index.js` is rebuilt, since the bundle is the artifact the runtime loads and the suite asserts against it.
Root C of three, and the one that had to answer carefully rather than
merely refuse. Both primitives here take a caller-supplied KEY rather
than a query, and both key spaces are enumerable: a chunk id is a
sequential integer, and a preference id is `pref-<topic-slug>`. A
distinguishable refusal over an enumerable key space is an existence
oracle over exactly the population the boundary hides, so a withheld read
is reported with the message an absent one produces, byte for byte -
the convention `expandHit` and `resources.ts` already held for owner
scope, now holding for one more rule.
`expandHit` asks on EVERY call rather than only when an argument arrived,
because the reach is never a caller argument and a boundary a caller opts
into is not a boundary. The four templated `osb://` readers ask through a
view built once at the switch, so a reader added later cannot be the one
that forgot. For `osb://topic/{slug}` the reach reaches the SELECTION,
not the result, for the reason the owner scope already documented there:
a topic resolves to exactly one preference, so filtering afterwards would
report the topic as having no rule whenever a reserved preference sorted
ahead of a readable one, hiding the readable rule from everybody.
Rather than a second copy of the machinery, `owner-scope-view.ts`'s
reference grammar - paths, bare ids, wikilink brackets, the `Brain/`
directory list an id resolves through - is extracted into
`artifact-ref-view.ts` and bound twice: once to ownership, once to reach.
A second copy would have been a second place for a directory to go
missing, and a missing directory silently turns a fail-closed promise
into a fail-open one. Both bindings delegate the per-path decision to the
single place their rule meets the filesystem, so a report-shaped surface
and a ranked one cannot drift.
Two defects found in code this change touches, fixed here and named:
- `osb://backlinks/{id}` scoped the backlink INDEX by owner but published
every ref the index returned, so a source artifact the caller may not
see was still named once the target was visible. Both rules now filter
the refs as well as the target.
- `applyVisibilityScope` was about to become the second spelling of "may
this reach read this path". The decision is `isPathReadableAtReach`,
beside `isPathOwnerVisible`, and the ranked filter, the drill-down and
the reference views all call it rather than restating it.
`ArtifactRefView.filtersNothing` exists so the rendered log day, whose
filtered form is a split and a rejoin rather than the original bytes,
stays verbatim when no rule is live.
`TRANSPORT_REACH` is registered in the four-piece vocabulary census and
its population equality bumped, which is what that equality is for.
Schema 12 adds `documents.visibility` and, more to the point, BACKFILLS it. A column added without one is NULL for every existing row, and the only fail-closed reading of "unmeasured" is "reserved" - so the shape the consultant recommended, treating a missing value as private and calling the legacy population self-healing, is a total search blackout on upgrade for every vault, including the overwhelming majority that never used the field. The frontmatter is already in the index, as its own chunk at `chunk_index = 0`, so the backfill is one table scan and a parse per document with no vault walk and no operator action. The design's open question, now measured rather than guessed: the backfill runs INSIDE the migration transaction. Over 5000 indexed documents it takes 86 ms - about 17 microseconds a document, against 8.9 seconds to index the same corpus - so the cost was never the deciding factor. Atomicity is: a half-filled column leaves measurable rows reading as unmeasured, which is the blackout the backfill exists to prevent, and a crash mid-pass outside the transaction would leave exactly that. The column is what the INDEX knows, and deliberately not the read boundary. The design proposed moving the search root onto it to save a frontmatter read per candidate; verified against the three roots this wave already enforces at, that would move the boundary from the live file to a snapshot that lags it by a whole index run, so a page reserved a minute ago would stay readable until the next one. The file decides and the column reports - the trade the design offered was cost against promptness at a boundary, and promptness wins. What the column earns, since a column nothing reads is a stub: - `anyVisibilityTagPresent` is now exact. It was a `LIKE '%visibility:%'` scan of every chunk-zero row, whose cheapest case was a vault that DOES use the field and whose worst case was the common one - and which read a frontmatter value quoting the key, or a body line spelling it, as a tagged vault. Both false positives are gone, and the two tests that pinned the imprecision as a documented tradeoff now pin its removal. - `countUnmeasuredDocuments` and `countRemoteReservedDocuments` are the populations `search check` reports rather than absorbs. One premise of `plan.md` refuted and recorded in the test's own docblock: an unterminated frontmatter block cannot land in the unmeasured state, because `chunker.ts`'s `readFrontmatter` drops the opening `---` and keeps the rest as body, leaving chunk zero byte-indistinguishable from that of a page with no frontmatter. It also does not matter - `parseFrontmatterText` reads an unterminated block on the real file as declaring nothing too, so the column and the live boundary agree. The unmeasured state is what it actually is: a document the index holds no chunk zero for. The indexer marks and never skips. A `continue` after the frontmatter parse would be a live defect: `seen.add` runs before the content is read, so the deletion sweep would not purge a skipped page's stale rows and the page would keep whatever chunks it already had.
The three roots are only a boundary if every caller goes through one, and until now that claim was prose. This is the sweep that checks it: every file under `src/mcp/`, `src/cli/` and `src/openclaw/` that opens a vault path with `node:fs` rather than through a root is found mechanically, must carry a row in `DIRECT_VAULT_READ_REGISTRY` with a closed category and a written reason, and the sweep fails in both directions - an unregistered file and a row naming a file that no longer reads that way. A row claiming `guarded` is checked against the file it is made about, so a decorative classification fails rather than reassures. It found one, which is the point of building it: `brain_clusters` listed `Brain/clusters/*.md` with `readdirSync` and `brain_bridges` read `Brain/proposals/bridges.md` with `readFileSync`, both by path and neither through any root. Both now ask the rule at the site of the read - a withheld cluster is dropped and nothing counts it, and a reserved proposals page answers exactly as an absent one. The other three sites are registered with their reasons: a count of files that discloses no path, a link-repair lane that must see every page to keep the graph it repairs consistent, and a CLI mirror the operator's own shell already answers at local reach. `src/openclaw/` joins the tool sweep's population. The previous census named the OpenClaw page walker as an un-swept surface and left it out, which meant the one place already identified as a gap was the one place the sweep could not have found it. The tool-population equality is unchanged at 43 and that is the measurement, not an oversight: the plugin registers the same tool names the MCP surface publishes, so its files sweep onto the registry rows that already exist. What the sweep cannot see is written down beside it rather than implied - a vault path built without `join`, and a read performed by a helper in `src/core/`, which is not a gap in the guarantee so much as a restatement of it. Two repeated literals in the code this touches are hoisted: `Brain/clusters` and `Brain/proposals/bridges.md` were each spelled twice per handler, once to build the absolute path and once to report the relative one, and the two spellings have to agree for the boundary check to be asking about the page it is returning.
…count itself The census shipped in v1.52.0 measured a gap and said so in the operator's own diagnostics: of the callable surfaces it enumerates, two consulted `visibility:` and the rest did not. This is the same instruments reporting the same measurement after the roots landed - the covered set is now ten MCP tools, seven CLI verbs and the four templated `osb://` readers, each with a written reason naming which root covers it, and every count still derives from `excludedCallableVisibilitySurfaces()` / `callableVisibilitySurfaces()` rather than being written down. Four surfaces needed work before they could be classified honestly rather than reclassified optimistically: - `brain_query` filtered nothing. Its topic mode now carries the reach into the SELECTION - a topic resolves to exactly one preference, so filtering afterwards would report a topic as having no rule whenever a reserved one sorted first - and its signals, log events and preference-mode lookup go through the reference view, with a reserved preference answering byte for byte as an absent one. - `brain_backlinks` and its CLI mirror published every ref the index returned. A ref NAMES its source artifact, so the target and the sources are both asked now, and a withheld target answers with the empty backlink document rather than a refusal that would prove it exists. - The CLI `brain query` and `brain backlinks` verbs state their reach at the call site. The verdict is admit-all because the caller is the operator's own shell - a decision these verbs make rather than a question they skip. `logEventRefs` was a private copy in `resources.ts` of the field list that says what a log entry discloses. It is `logEntryArtifactRefs` in `core/brain/log.ts` now, because the second consumer arriving is exactly how one of the two comes to disclose more than the other. The finding's sentence changed shape because the fact it stated did. It said `visibility:` was a caller-supplied view filter and not a privacy boundary, which was true and is not any more. It now reports what is still true: how many indexed documents reserve themselves against remote reads - the behaviour change an operator has to be able to size rather than infer from a search that came back shorter - how many enumerated surfaces still do not consult the field, and how many documents the index holds no frontmatter for, reported rather than folded into "declares nothing". The `index_store` row is kept and reworded rather than dropped. A reserved page's text is still in `chunks`, deliberately: excluding it would take an operator's own private notes out of their own local search, which is a product regression dressed as a hardening. What changed is that the index now records what it measured, and the row says plainly that the column reports while the live file decides.
The owner matrix drives every tool in the live table against a two-OWNER fixture. Its fixture carries no reserved page, it asserts nothing about the MCP `_meta` response member, and it drives no federated caller - so a page reserved against remote reads could have leaked through any of the surfaces it sweeps and every one of its assertions would still have been green. This is the second pass over the same enumeration. A SIBLING file, and the design left that open to be decided on evidence: the two axes cannot share one fixture. The owner matrix builds its contexts without a transport reach, which resolves to the narrowest, so the moment its fixture gained a reserved page every owner assertion would have had to say which reach it was asking at and its gate-off half would have been measuring two rules at once. The recipes are SHARED rather than copied. `tests/helpers/tool-probe- catalogue.ts` now holds the invocation for every code path of every tool, and both matrices import it - a second copy of "what is the whole surface" is the enumeration defect the first matrix was built to remove. The owner matrix keeps its own classification and its 264 tests stay green over the move. Every probe goes through the real JSON-RPC `tools/call` path with a progress token attached, and asserts over the WHOLE serialised response. That covers the four channels a marker could ride out on in one check: `structuredContent`, the rendered `content[].text`, the `message` a throw produces, and the `_meta` member the progress refusal travels in - which no prior sweep looked at and which is live on this transport. Two buckets, both executed, because a blanket "no tool may name a reserved page" would have been a claim this wave does not deliver - and the first run proved it, with twenty-eight recipes still naming one. A tool the registry calls covered must withhold; every other recipe's verdict is measured in `STILL_NAMED_AT_REMOTE` and asserted in both directions, so a recipe leaving that list is the wave that covered it and one joining it is a regression. One real leak found and fixed on the way: `brain_clusters operation=run` reported community membership filtered by owner scope alone, so a community naming a reserved page was reported to a remote caller in full. Both rules are ANDed now, a community either rule withholds is dropped whole rather than trimmed - a community of seven reported as four is not a narrower true finding but a false one - and detection stays vault-wide, because clustering the visible half of a link graph would produce different communities per caller and write them over each other. Named narrowing in that same handler: `removed` is now withheld whenever ANY rule is live rather than only under an owner scope. A removal names a cluster note whose id is derived from its seed page's path, so at remote reach it can name a seed the `communities` list has just withheld, and the field's own stated reasoning - its subject cannot be resolved - is what makes withholding the only answer.
`buildCacheKey` enumerates every result-affecting option explicitly, and `transportReach` became one when the reserved-token rule moved into `applyVisibilityScope` - it decides which pages are in the pool at all. It was not in the key, so a `local` outcome and a `remote` outcome collided on one row of the shared `query_cache` table, which the operator's own CLI and the HTTP MCP server both write to. Within the TTL and the same corpus generation, whichever warmed the row first served the other: an operator's `o2b search` handed a remote caller the reserved pages it had just been denied, and the reverse poisoned the operator's own shell with a filtered set. Folded in RESOLVED rather than raw, so an absent reach and an explicit `remote` key identically - the pipeline already reads them as the same thing, and a third key for "nobody said" would partition the cache without partitioning the answer. Folded in unconditionally rather than only for `remote`, because every row written before this boundary existed was computed without it and must not be served under it; the one-time miss is the correct reading of a row whose filter never ran. The `?? remote` this needed is now `resolvedTransportReach` in `core/graph/transport-reach.ts`. It was spelled six times across the tree - two options bags, two context readers, the server constructor and the resource reader - which is one rule with six spellings and five places to forget. `contextReach` is that function under the name the MCP seam already uses.
`applyVisibilityScope` was carrying two rules, and the retrieval trail measured its baseline in front of both. `preVisibility` was the row count before the reach rule ran, so `scope-filters-dropped-rows` published `dropped` and `before` for pages the caller was withheld from - on a surface that is not gated behind `explain` and rides out on every `brain_search` response. A remote caller searching a term nobody public wrote about got zero results and a trail saying two rows were removed, which is the existence oracle the boundary exists to close, and it is enumerable one query term at a time. The two rules are separate functions now. `applyReachFilter` answers "may a caller at this reach see the page at all" and runs first; `applyVisibilityScope` answers the caller's own scope question and runs after, so it can still only narrow. The trail's baseline is taken between them, which makes the number it publishes exactly what the caller's own arguments removed. The assembly's one-shot backfill needs the other count, and gets its own field rather than the trail's. A reserved page crowding a narrow rank cap has to widen the window the same way a tagged page already does - otherwise a remote search over a vault with six reserved matches and four public ones returns nothing at all, which the existing backfill test caught. `prePoolFilters` is consumed to fetch more rows and never reported, so it discloses nothing; `preVisibility` is published verbatim and therefore counts the reach rule out. The trail's counterweight is pinned too: a scope the caller DID request is still reported as narrowing.
Relation polarity runs AFTER the pool filters and does not only reorder: `applyRelationPolarity` resolves typed `superseded_by` edges to documents that were never in the filtered pool, fetches their representative chunks off the store, and returns `[...pool, ...pulledIn]` with full path, title and content. Nothing between that append and the outcome re-asks any filter, so a public page declaring a reserved successor was a route to that successor's body at remote reach - and the pool filter could not have caught it, because the row was not in the pool to be caught. The lane is on by default. Root A is asked again over the whole pool rather than over the pulled-in rows alone: the verdict is idempotent and every already-filtered row answers off the shared frontmatter cache, so one spelling of the rule costs less than a second one that tracked which rows were new. Recorded at the seam rather than fixed with it: the caller's `visibility` scope and its `agentScope` have the same gap here and predate this boundary. A pulled-in successor bypasses `applyVisibilityScope` and `applyAgentScope` exactly as it bypassed the reach rule. Closing those means re-resolving the caller's whole filter set after a post-rank phase, which is a change to the ownership boundary rather than to this one, so it is written down at the site instead of being silently carried. Pinned in both directions: the reserved successor stays out of a remote window and none of its body rides out on the surviving row, and the same edge at a reach that reaches the page still pulls it in.
`runRecallBenchmark` pinned `TRANSPORT_REACH.local` on every search it made, under a comment calling itself an internal measurement lane. It is not one: `dataset` is a required argument of `brain_benchmark` and `brain_tune`, both callable over any transport, and neither handler consulted the reach the transport minted. A reach constant named at a call site that has a caller is the defect this wave is about - the site reads as decided when what it decided was to opt out. The reach is an option now, resolving to the narrowest when absent. The two MCP handlers pass `contextReach(ctx)`, the CLI benchmark and tune verbs pass the CLI's own reach, and the rerank eval gate - which has no caller at all - names `MAINTENANCE_LANE_REACH` so its claim is the same claim every other lane makes. PREMISE REFUTED, and recorded rather than dropped. The finding was reported as an oracle over `visibility:`-tagged pages: submit a dataset naming a reserved path, read `hit` / `rank` / `expectedFound`, then binary-search the body through `answerContained`. Measured against the live code, it is not. `search()` also applies the caller's visibility SCOPE, the benchmark passes none, and a tagged page is therefore outside every benchmark run at every reach - a tagged-page probe scores absent with this fix reverted, which is why the test that would have proved it proves nothing. What the pin actually reached is the UNMEASURABLE page: a document still in the index whose file was deleted, renamed or made unreadable since the last run. The reach rule substitutes the reserved token for it and denies at remote; the caller-scope rule reads its empty frontmatter as untagged and keeps it. That difference is the whole observable effect, and it is what the three pinned tests measure - including the reverted-code run that fails one of them. `brain_eval`'s registry row moves from excluded to covered, and its reason is rewritten: "returns metrics, not note bodies" was the wrong ground for excluding a surface whose metrics are computed over a corpus the caller names.
The three roots have to agree about the one input that has no answer,
and they did not. Roots A and C substitute the reserved token for a page
whose file cannot be READ - `isPathReadableAtReach` reads the verdict
`readCachedFrontmatterEntry` carries, on the stated ground that an
unreadable visibility claim is not the absence of one. Root B decided off
the parsed map alone, and the parser resolves an unreadable file to `{}`,
which is byte-indistinguishable from a page that declares nothing. So
`listVaultPages` read it as untagged and listed it at remote reach: the
same rule, spelled twice, failing in opposite directions on the same
page.
The routine trigger is not exotic. A page deleted, renamed, moved to a
failing mount or chmod'd between one call and the next lands here, and
`second_brain_query` reaches it with no agent scope - the branch where
the handler does not even collect the notices that would have told it
the read failed. Toggling a reserved page's readability was itself the
oracle: readable meant withheld, unreadable meant listed with its path
and its filename-derived title.
The walk now carries the read verdict beside the parsed map and
substitutes the same token those roots do. `WalkedPage` is internal to
the module - the flag exists to decide the walk's own filter, and
`VaultPage` stays the shape every caller already consumes. The
substitution constant is spelled from the same exported token as its
twin in `result-filters.ts`, so the two cannot drift into meaning
different things.
Pinned in both directions, with the reverted code failing the first: an
unreadable page is withheld at remote, kept at local - where the caller
can open the file itself and hiding a stale row from the operator who has
to fix it helps nobody - and a readable untagged page is untouched at
both.
Both refusals claimed byte-identity with the absent form in their own
comments, and neither had it.
`brain_query mode=preference` built its refusal as
`new BrainNotFoundError("preference not found: " + arg)`. That
constructor formats the sentence itself, so the withheld page answered
"...no preference or retired entry found for id 'preference not found:
pref-x'" while an absent one answered "...for id 'pref-x'". The owner
branch three lines above had the same shape and predates this wave; both
now pass the caller's own argument, which is what `queryByPreference`
throws for a page that does not exist.
`osb://preference/{id}` echoed the RAW id on the withheld branch while
the absent branch rethrows what `queryByPreference` was handed - the
NORMALISED one. The reader accepts the bare slug as well as `pref-` and
`ret-`, so for that key shape the two diverged: `osb://preference/acme`
answered "...for id 'acme'" when the page existed and was withheld, and
"...for id 'pref-acme'" when it did not. The presence of the prefix was a
one-bit existence oracle over the whole reserved population, and the
prefixed key shape - the only one the parity test exercised - is exactly
where the two strings happen to coincide.
Both key shapes are pinned now, on both surfaces, comparing the messages
with the caller's own argument removed. Reverting either fix fails its
test.
…he rule in both modes Two surfaces where the boundary held on the row and leaked around it. `brain_query mode=topic` filtered its signals and log events and then reported the count it had before filtering. That number is written to the vault's recall-telemetry log and handed back verbatim by `brain_recall_telemetry operation=list`, folded into `summary.total_results` as well - so a caller that saw an empty topic could read, through a second tool, how many rows it had been withheld from. The `since` branch beside it already recounted after filtering; the topic branch is now the same shape, filtering once and using the filtered lists for the response and the count alike. `brain_bridges` was classified COVERED off its `list` mode alone, which was the registry making a claim the tool did not hold. `discover` returned `discoverBridges()`'s proposals verbatim, each naming two pages by vault-relative path off the vec index - and the index keeps reserved pages by design, because `documents.visibility` reports what was measured rather than excluding what is held. Detection stays vault-wide and the shared `Brain/proposals/bridges.md` is still written unfiltered: a bridge proposed from the visible half of a link graph would differ per caller, and the file is one file. The rule is applied to what the CALLER is told, which is the shape `brain_clusters run` already uses, and a proposal is dropped WHOLE when either end is withheld - a bridge reported with one end missing is not a narrower true finding but a false one. `scanned_candidates` names no page and is the same measurement at every reach, so it is reported as measured; that is pinned too. The registry row says both modes now, and says what each one does. Reverting either fix fails its test.
…on shape Both rules ask the same question through `artifactRefView`: "does this row name an artifact the caller may not see". A reference the resolver cannot place is read as naming nothing and lets its row through - so every spelling it could not place was a fail-open on a rule whose whole posture is fail-closed. Three of them were live. SPELLING. `unbracket` took the brackets off and left the decoration on, with a comment saying the id-resolution step would handle it - it does not handle it, it fails open on it. `pref-x|alias`, `pref-x#Raw` and `Brain/preferences/pref-x.md|the rule` end in neither `.md` nor an id any Brain directory holds, so they resolved to null and passed. These are not hypothetical: `brain_apply_evidence` documents the aliased and line-ranged forms for its own `artifact` argument. The decoration comes off now, through a stripper split out of `stripBasenameDecoration` - the half that removes `|` and `#` and NOT the half that collapses folders and drops `.md`, which is the fail-open the docblock was right to refuse. Cutting decoration only ever shortens the target, so a reference that resolved before still resolves. SHAPE. `logEntryArtifactRefs` read four named keys and only when their value was a `string`. `BrainLogEntryPayload` is `string | ReadonlyArray<string>` and `parseLogDay` produces the array whenever a key repeats or uses the sub-bullet form, so a multi-artifact reference answered `undefined` - which the view reads as "names nothing". Arrays are flattened now. ENUMERATION. The four names were `path`, `preference`, `signal`, `artifact`. A census of every `appendLogEvent` body in this tree finds artifact references in `target`, `subject_a`, `subject_b`, `successor`, `predecessor`, `source`, `source_path`, `note`, `file`, `files`, `retired`, `superseded_by` and `conflicts` as well, so the list was a sample of what a row discloses rather than a measurement of it. Every string in the payload is offered now. Enumerating a payload whose writers are open-ended is the defect, not the particular keys that were missing - and offering too much is safe in the way offering too little is not, because a string that resolves to no artifact is read exactly as an absent key is. The widening pays for itself with a per-reference memo beside the existing per-path one: rows name the same artifact repeatedly, and the id-shaped miss costs one `existsSync` per Brain directory. `brain_backlinks` gets the same rule over its `unparsed` list, which was published unfiltered. Each entry names its source artifact by id, so it disclosed a reserved page through a call that never mentioned it. Thirteen spellings and shapes are pinned; six fail against the old code.
…verstating itself Three of this branch's own claims did not hold, and all three are claims about honesty rather than about enforcement - which is the worse kind to leave standing. TWO SURFACES OUTSIDE THE DENOMINATOR. The census sweeps a file only when that file imports a note-content primitive directly, and both `brain_hygiene` and `brain_skill_proposals` reach page content through a `src/core/` helper instead. So neither was in the population, neither had a registry row, and neither was counted in the number `search check` reports to operators - while both were returning reserved page paths at remote reach. `brain_hygiene`'s freshness detector walks with `MAINTENANCE_LANE_REACH` and puts the page's path in `targets` with a title stating a fact about it; `brain_skill_proposals page_candidates` returns a path and title per page in `admitted` and in `skipped`, the latter with a `detail` saying why the gate turned it down. Both are filtered at the handler seam the owner rule already sits on - so a hygiene detector registered after this one inherits the rule - and both recompute their own count from the visible rows, because a corpus size taken before the filter states how many rows were withheld. The vocabulary gains the two helpers, which brings three more tools in on the file-level rule; each is classified with what it actually discloses rather than waved through as over-inclusion, and `brain_procedural_memory` is excluded while SAYING that it returns `sourcePath` and `title`. A SWEEP THAT MOSTLY SWEPT NOTHING. The matrix drives every recipe of every registered tool and asserts the marker is absent at remote reach, anchored by four recipes asserted to reach it locally. Measured: most recipes never put the marker in front of a local caller either, so most of those assertions are coverage of the enumeration rather than of the rule. The count is now taken as the sweep runs - on each recipe's own fresh fixture, after its assertion, because several recipes write to the vault - and bounded in both directions, with a guard that fails by saying the sweep was skipped rather than reporting a bound nobody measured. A DIAGNOSTIC THAT WENT QUIET WHEN IT MATTERED. The honesty finding was gated on `tagged`, and `unmeasuredDocumentCount` is only rendered inside it - so an index whose column is NULL for every row, which is exactly the state that count exists to surface, printed no line at all. It reports whenever there is something to be honest about: a tagged page, or a population the index could not measure. Also: `peekVisibilityTagPresence` had no production caller once the census subsumed it, and was kept alive by its own test - a passing suite over a path the product no longer walks. Removed, with the test repointed at the census. Two docblocks orphaned by function moves are put back over the functions they describe, in `cards.ts` and `resources.ts`. The registry header no longer says this module makes no enforcement change, and the census header no longer names `src/openclaw/` as a blind spot it has since swept - a closed blind spot left standing mis-sizes the remaining work in the opposite direction from a missed one. The honesty-finding test asserted `total > excluded`, which only says the covered half is at least one; it asserts against the two surfaces the wave inherited.
…lker `openclaw/index.js` is a checked-in build artifact produced from `src/openclaw/index.ts` by `bun run build:openclaw`, and it bundles `listVaultPages`. Making root B fail closed on an unreadable page changed that function, so the committed bundle no longer corresponded to its source - and the OpenClaw runtime mints `remote`, which is precisely the reach the fix is about, so the plugin was running the version of the walker that admits a page it cannot measure. Verified in both directions: rebuilding at the commit before that fix produces no diff, so the artifact was in sync when it arrived and this rebuild is the whole of what the fix owed it.
The test titled "the _meta channel is live on these probes" asserted
`expect(response).toContain("_meta")` over the serialised envelope. Every
probe sends `params._meta.progressToken` itself, so that substring was in
the request the probe had just made - the assertion matched the echo and
would have stayed green over an empty or absent `result._meta`, which is
the retirement it exists to catch.
It reads `result._meta` off the parsed response now and requires the
namespaced progress key in it. Emptying the member in `progress.ts` fails
the test.
The branch arrived at the same 146 warnings as `main` and my fix commits took it to 151: two shadowed locals, one dangling-underscore property access and one await-in-loop, all in tests added during the self-review. Back to 146, which is the number to hold rather than a number to explain.
`attaching an observer changes nothing the pass writes` failed about one
run in three under a loaded suite, and never in isolation. The whole
difference was the snapshot entry's `size_bytes` - 9446 against 9456.
The test already excludes `Brain/.snapshots/` because a tar carries the
mtimes of the tree it was taken from and the harness wrote the two trees
at different instants. That exclusion was incomplete: the snapshot log
entry republishes the archive's SIZE into `Brain/log/<date>.{md,jsonl}`,
which the assertion does compare, so the excluded nondeterminism came
back through a file that was still in scope. Whether it tripped depended
on whether the copy straddled a second, which is why load decided it.
Normalised rather than excluded. Dropping the day log would take the
pass's main authored artifact out of the comparison, which is most of
what this test exists to check; replacing one field leaves run_id,
reason and channel still compared.
Pre-existing, not introduced by this branch: the file is byte-identical
to main, and the flake reproduces against main's source at the same rate
(9445 against 9441). Verified 40 consecutive loaded runs green, and a
mutation appending one byte to the day log is still caught.
Phase 5 for the visibility-enforcement wave: the CHANGELOG entry, the README lead, the two reference docs a user consults for the surfaces this wave changed, and the version bump that rides in this pull request per CLAUDE.md rather than after it. CHANGELOG 1.54.0 leads with what the release actually costs a vault that uses the field - pages that read over MCP today stop reading - and with the guarantee for the vaults that do not: the schema-12 backfill, not an assumption. Both source cards' refuted premises are recorded as findings rather than designed around silently, along with the correction to the wave's own stated risk about indexRevision. Every number in it is the registry's or the index's own measurement: 24 of 76 callable surfaces covered against 3 of 71, and the 52 that remain named as the honest remainder rather than left implied. README's "What is new" leads on 1.54.0 and demotes 1.52.0, which had been the lead across two releases. Its one now-false clause is corrected rather than deleted: the standing census did truthfully document that the field was a view filter, at the time, and it is the coverage map this wave built the enforcement against - so the sentence moves to the past tense and says what became of it. Two reference docs beyond what design.md named, because shipping an enforcement boundary with no reference documentation would be its own dishonesty. docs/mcp.md gains three Safety notes in the established "since vX" shape: the reservation, the transport-minted reach and the -32602 caller-supplied-reach refusal with its structured data, and the absent-versus-withheld parity with what the local bypass does and does not prove. docs/cli-reference.md gains a Search subsection stating that every verb in this CLI runs at local reach and why, that --visibility can only narrow, the fail-closed reading of an unreadable page in both directions, and the column that reports while the live file decides. package.json 1.53.1 -> 1.54.0, propagated with scripts/sync-version.ts; --check reports ok on all seven mirrored manifests. Minor rather than patch: the release adds capability and changes what a remote caller is answered. Verified rather than assumed - fmt:check clean over 2269 files, lint at the same 146 warnings and 0 errors as main, the version, bundle and docs suites green at 49 pass / 0 fail, and build:openclaw produces no diff, so the checked-in bundle still corresponds to its source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
solaitken
enabled auto-merge (squash)
August 27, 2026 18:16
|
Important Review skippedToo many files! This PR contains 118 files, which is 18 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (118)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
itechmeat
approved these changes
Aug 28, 2026
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
visibility: privatestops being decorative. One reserved token is now denied by default at remote reach, enforced at the three roots every read surface reaches page content through - the ranked search pipeline, the page walker, and the by-path / by-chunk-id read primitives - with an architecture sweep proving mechanically that no fourth root exists. The reach that decides is a transport fact minted by the server (stdio/CLI = local, non-loopback HTTP = remote); a caller-supplied reach argument is refused by name. A withheld page answers byte-for-byte as an absent one, so the boundary is not an existence oracle. Coverage moves from 3 of 71 note-returning surfaces to 24 of 76, ando2b search checkreports the honest remainder.Why the boundary holds
flowchart LR A[Request arrives] --> B{Transport minted reach} B -->|stdio / CLI| C[local - operator's own process] B -->|non-loopback HTTP| D[remote] B -->|caller supplies reach| E[Refused by name -32602] C --> F[Reserved pages readable] D --> G{Three enforced roots} G --> H[Search pipeline] G --> I[Page walker] G --> J[Key-addressed reads] H --> K[Withheld = absent, byte for byte] I --> K J --> K K --> L[Census proves no fourth root]What ships
isRemotelyReadable+ reserved token ingraph/visibility.ts; callervisibilityargument can only narrow, never lift.TransportReach(local|remote) minted at the transport, refused from callers with structured-32602data.documents.visibilitymeasured column with in-transaction backfill (86 ms / 5000 docs) - no operator reindex required.brain_clusters run, hygiene/skill-proposals paths) are fixed.Review and QA
Test plan
env HOME=$(mktemp -d) bun test tests/- 12069 pass / 0 failbun run typecheck/bun run lint/bun run fmt:check- cleanbun run sync-version:check- 1.54.0 across all seven manifestspython3 -m unittest discover -s tests/python- 146 OK