Skip to content

fix(core): the rules export must contain the rules the Brain serves - #187

Merged
bejranonda merged 1 commit into
mainfrom
fix/exporter-user-scope-reach
Aug 1, 2026
Merged

fix(core): the rules export must contain the rules the Brain serves#187
bejranonda merged 1 commit into
mainfrom
fix/exporter-user-scope-reach

Conversation

@bejranonda

Copy link
Copy Markdown
Owner

Closes the open finding from #186: the default "Download rules bundle" shipped a strictly narrower set than kra.ts injects into your sessions.

The fix, and why it isn't the migration I originally proposed

I suggested "migrate the three V1 call sites to V2". Checking before executing showed that would have been wrong:

ownerUserId anchoring
V1 buildKnowledgeWhere base = {ownerUserId} ANDed over every branch — strictly your own rows
V2 project arm {visibility:'project', ownerProjectId}no owner predicate, by design (Phase-4 org sharing)

Migrating a personal surface to V2 would therefore also start returning teammates' rules — a product decision about team sharing, shipped inside a change labelled "finish the migration". Exactly the kind of thing that's hard to spot in review because the diff looks like cleanup.

So: an owner-anchored includeUserScopeAcrossProjects opt-in on V1, always enabled by the exporter. Not a parameter — there's no reading of "export my rules" that wants less than you're served.

Scoped to the exporter on purpose

Dashboard and graph keep the project-scoped view. They're browsing surfaces where project focus is the point — the same retrieval-is-wider-than-listing distinction KNOWLEDGE §12.19 already draws. A rules bundle is different in kind: it's the agent's configuration, so it has to match what the agent gets.

The general rule, now in GUIDELINES

Migrate a surface to V2 when you want org visibility there; add an owner-anchored opt-in to V1 when you only want the caller's own rows to reach further.

Test plan

  • TDD — four tests written first, watched the two behavioural ones fail. 62/62 green across both scope-filter suites afterwards.
  • Tests pin: closed by default · opened when opted in · no-op under DataScope: "all" (which already returns everything owned) · ownerUserId present both on the outer AND and inside the new disjunct, so it's safe read in isolation.
  • Verified the V1/V2 owner-anchoring difference in the source before choosing the approach, rather than assuming the migration was the clean path.
  • 87/87 across the three suites that run standalone.
  • exporter.test.ts relies on CI — it imports @brain/db, so it can't load under standalone vitest in this checkout. That's the suite most relevant to this change, so CI is the real gate here.
  • Reviewer: confirm you agree a rules bundle should follow the user, not the project. That's the one judgement call in this PR; the code change is small either way.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Rqj5y9fT3XPUqnqKyustp9

Closes the open finding from #186: "Download rules bundle" shipped a
strictly narrower set than kra.ts injects.

Two things are called "scope" and the distinction is the fix. Under
DataScope "all" the V1 helper already returns everything the caller
owns. The gap was DataScope "project" — buildRulesBundle's default —
where it resolved on ownerProjectId alone and never consulted
Knowledge.scope, so it could not filter to a project WHILE admitting
the caller's user-scoped rows from elsewhere.

Fixed with an owner-anchored includeUserScopeAcrossProjects opt-in on
V1, always enabled by the exporter. Not exposed as a parameter: there
is no reading of "export my rules" that wants less than you are served.

Deliberately NOT the V1->V2 migration I originally suggested. Checking
before executing showed V2's `visibility: 'project'` arm carries no
ownerUserId predicate — that absence is what makes Phase-4 org sharing
work — so migrating a personal surface to it would ALSO start
returning teammates' rules. That is a product decision about team
sharing, not this defect, and it would have shipped inside a change
labelled "finish the migration".

Dashboard and graph keep the project-scoped view on purpose. They are
browsing surfaces where project focus is the point, consistent with
KNOWLEDGE §12.19's retrieval-is-wider-than-listing rule. A rules bundle
is different in kind — it is the agent's configuration.

Tests pin all four properties: closed by default, opened when opted in,
no-op under DataScope "all", and the ownerUserId anchor present both on
the outer AND and inside the new disjunct so it is safe read alone.

GUIDELINES now records the general rule this produced: migrate a
surface to V2 when you want org visibility there; add an owner-anchored
opt-in to V1 when you only want the caller's own rows to reach further.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqj5y9fT3XPUqnqKyustp9
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@bejranonda, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 30 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1c1509f7-59c6-4bb0-b7a3-9a59ce996af6

📥 Commits

Reviewing files that changed from the base of the PR and between 0b5ea96 and 2ab80b1.

📒 Files selected for processing (6)
  • docs/GUIDELINES.md
  • docs/KNOWLEDGE.md
  • docs/KNOWN_ISSUES.md
  • packages/core/src/__tests__/scope-filter.test.ts
  • packages/core/src/exporter.ts
  • packages/core/src/scope-filter.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@bejranonda
bejranonda merged commit e41d260 into main Aug 1, 2026
7 checks passed
@bejranonda
bejranonda deleted the fix/exporter-user-scope-reach branch August 1, 2026 10:41
bejranonda added a commit that referenced this pull request Aug 1, 2026
…188)

export surface is empty for everyone

Measured the #187 fix against the live corpus after deploying, and the
measurement contradicted my own framing.

The fix is correct: the base clause now admits 183 rows instead of 146
for a Brain Platform-scoped export, matching retrieval's reach. But
buildRulesBundle ALSO filters tags: { has: "rules-export" }, and zero
rows carry that tag for any user — so the bundle is empty regardless
and nothing user-visible changed. The PR described a live symptom;
there isn't one. Latent-correct, not observable. Corrected in place.

That measurement turned up something larger, now recorded as open:
"Download rules bundle" returns nothing for everyone. Exactly one
production path writes the rules-export tag — autoskill.ts:801, on
proposal APPROVAL — and all 4 proposals on the reference instance are
pending. The demo seed tags 4 rows, but that is a dev fixture.

Not obviously a bug: invariant 8 requires approval before autoskill
writes anything, so an unused queue legitimately yields an empty
export. But it means the export surface has never been exercised by
real usage, and it raises a product question worth answering
deliberately — should "export my rules" mean approved autoskill rules
only (today), or the rules the Brain actually serves you, which is what
the button label implies?

Sixth imprecision of this session caught by verifying rather than
remembering. The pattern holds: the finding was real, the claim about
its impact was not.


Claude-Session: https://claude.ai/code/session_01Rqj5y9fT3XPUqnqKyustp9

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant