docs(claude): the tenant guard shipped — stop saying nothing catches it (#1355) - #1357
Conversation
CLAUDE.md:131 still read "a cross-tenant leak that nothing catches. Adding a guard is tracked in #1226." The guard shipped in #1351. That is worse than an ordinary stale doc because CLAUDE.md is loaded as ground truth by every agent session, and the claim is false in a direction that changes behaviour: an agent reading it either duplicates the guard or reasons more defensively than the code requires. What I deliberately did NOT do is soften the mandate. The first two clauses are load-bearing and stay verbatim: there is genuinely no ORM- or middleware- level enforcement, and the filter must still be written per query, in the route. The ratchet is a test-time safety net; an agent that believes the ORM scopes queries will write an unscoped one, which is a worse outcome than the stale sentence. The guard pins the reference by PATH, not by phrasing, so a future rewrite is free to reword but not to drop the pointer — and asserts the path exists, so renaming the test without updating the doc fails too. Proven in both directions: removing the path fails, and softening "not runtime enforcement" to "handled for you" fails. Closes #1355 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
WalkthroughUpdates multi-tenancy guidance to reference the shipped tenant-scope test and adds documentation-accuracy checks for its path, existence, and required wording. ChangesTenant documentation accuracy
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/lib/__tests__/docs-accuracy.test.ts`:
- Around line 114-123: Update the documentation accuracy test around the
existing CLAUDE.md assertions to explicitly verify that the stale “tracked in
`#1226`” wording is absent, while preserving the current path and caveat checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: eef091e5-56a9-44ab-8e79-b7e7a2bd4cce
📒 Files selected for processing (2)
CLAUDE.mdapp/src/lib/__tests__/docs-accuracy.test.ts
| const doc = readDoc("CLAUDE.md"); | ||
| const guardPath = "app/src/lib/db/__tests__/tenant-scope.test.ts"; | ||
| expect(doc).toContain(guardPath); | ||
| expect(existsSync(resolve(REPO_ROOT, guardPath))).toBe(true); | ||
|
|
||
| // The mandate itself is load-bearing and must survive any rewording: the | ||
| // ratchet is a test-time safety net, NOT runtime enforcement. An agent | ||
| // that believes the ORM scopes queries will write an unscoped one. | ||
| expect(doc).toMatch(/per query, in the route/); | ||
| expect(doc).toMatch(/not runtime enforcement/); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that the stale tracking claim stays removed.
The PR objective explicitly removes the “tracked in #1226” wording, but this test only checks that the new path and caveats exist. Reintroducing the stale claim would therefore pass CI.
Proposed assertion
expect(doc).toMatch(/per query, in the route/);
expect(doc).toMatch(/not runtime enforcement/);
+ expect(doc).not.toMatch(/tracked in `#1226/i`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const doc = readDoc("CLAUDE.md"); | |
| const guardPath = "app/src/lib/db/__tests__/tenant-scope.test.ts"; | |
| expect(doc).toContain(guardPath); | |
| expect(existsSync(resolve(REPO_ROOT, guardPath))).toBe(true); | |
| // The mandate itself is load-bearing and must survive any rewording: the | |
| // ratchet is a test-time safety net, NOT runtime enforcement. An agent | |
| // that believes the ORM scopes queries will write an unscoped one. | |
| expect(doc).toMatch(/per query, in the route/); | |
| expect(doc).toMatch(/not runtime enforcement/); | |
| const doc = readDoc("CLAUDE.md"); | |
| const guardPath = "app/src/lib/db/__tests__/tenant-scope.test.ts"; | |
| expect(doc).toContain(guardPath); | |
| expect(existsSync(resolve(REPO_ROOT, guardPath))).toBe(true); | |
| // The mandate itself is load-bearing and must survive any rewording: the | |
| // ratchet is a test-time safety net, NOT runtime enforcement. An agent | |
| // that believes the ORM scopes queries will write an unscoped one. | |
| expect(doc).toMatch(/per query, in the route/); | |
| expect(doc).toMatch(/not runtime enforcement/); | |
| expect(doc).not.toMatch(/tracked in `#1226/i`); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/lib/__tests__/docs-accuracy.test.ts` around lines 114 - 123, Update
the documentation accuracy test around the existing CLAUDE.md assertions to
explicitly verify that the stale “tracked in `#1226`” wording is absent, while
preserving the current path and caveat checks.
|
…t the new text exists (#1359) CodeRabbit's finding on #1357, which I merged before reading — my own rule, broken the same way as on #1331 earlier in this session: I checked the comment count and merged in the same command. The finding is right. The test asserted the new path and the two load-bearing caveats are present, but nothing stopped someone reintroducing "Adding a guard is tracked in #1226" alongside them. CLAUDE.md would then contradict itself, which is worse for a reader than either version alone — and worse for an agent, which has no way to tell which sentence is current. Now asserts both directions: the new text present, and both stale phrasings absent. Proven by reintroducing the claim next to the corrected wording and watching the suite go red. Refs #1355 Co-authored-by: alfredorubin96 <alfredo.rubin@neotechnology.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>



What
CLAUDE.md:131still read:The guard shipped in #1351.
That's worse than an ordinary stale doc, because CLAUDE.md is loaded as ground truth by every agent session — and the claim is false in a direction that changes behaviour. An agent reading it either duplicates the guard or reasons more defensively than the code requires.
What I deliberately did NOT do
Soften the mandate. The first two clauses stay verbatim: there is genuinely no ORM- or middleware-level enforcement, and the filter must still be written per query, in the route.
The ratchet is a test-time safety net, not runtime enforcement. Rewriting this to imply the problem is solved would be worse than leaving it stale — an agent that believes the ORM scopes queries will write an unscoped one. The new wording says both things explicitly.
The guard
Pinned by path, not by phrasing, in the existing
docs-accuracy.test.ts(the suite that already guards CLAUDE.md, from #1235). So a future rewrite is free to reword but not to drop the pointer — and it asserts the path exists, so renaming the test without updating the doc fails too.It also pins the two load-bearing phrases, which is the less obvious half: the risk here isn't only that the reference goes stale, it's that someone "tidies" the caveat away.
Proven in both directions:
Closes #1355
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Tests