Repository navigation
Keep .knowledgebase writes to cairn: file-tool hook and frontmatter check (PROF-16469) - #946
Conversation
.knowledgebase/ is written only by the cairn skills, which install memories through `cairn memorize`. A PreToolUse hook now denies Edit, MultiEdit, Write and NotebookEdit into it, in any checkout of this repository and through relative paths, `..` or symlinks. Like the gradlew hook, it loads its policy from origin/main and allows everything until the policy is there. A Pi adapter covers Pi's edit and write tools; Pi has no notebook tool. Shell writes bypass the hook, so check-knowledgebase.py, run by qualityChecks, checks every tracked memory for cairn's required frontmatter. AGENTS.md now carries cairn's own block verbatim, so cairn's session-start rewrite changes nothing, and states the rule in one line after it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…OF-16469) Guard: - Pi drops a leading `@` and expands `~` before it writes, and may run in a subdirectory; the adapter strips `@`, finds the hook at the checkout's top level, and the hook expands `~`. - The hook and policy match `.knowledgebase` case-blind, as macOS's default file system does. Frontmatter check: - Parse exactly as cairn does, so a memory cairn accepts is never rejected; a tracked file missing from the working tree is reported, not a crash; also check cairn's source form and dead-* kind rule. Memories are cairn's content and travel with the change they came from: the pre-pr-review skill tells the reviewer to skip .knowledgebase/ and fixes check-knowledgebase.py violations through cairn; the review marker still covers the files. AGENTS.md spells out the review skip. Tests: T7 in the quality suite for check-knowledgebase.py, the qualityChecks wiring, and a guard case for each gap above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ba1557f42
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Scan-Build Report
Bug Summary
Reports
|
|||||||||||||||||||||||||||||||||||||||||||||||||
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
On case-insensitive filesystems, an absolute memory path with different checkout-root casing bypasses both Claude and Pi guards. The frontmatter check does not catch edits that preserve valid metadata.
🤖 Bits Code Review · Commit 4ba1557
CI Test ResultsRun: #42281409207 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled ❌ Failing tests
Retried 1 of 32 cells. Failed Jobs
Summary: Total: 32 | Passed: 31 | Failed: 1 Updated: 2026-11-06 20:05:36 UTC |
…limiters (PROF-16469) - On a case-insensitive file system, `/x/REPO/.knowledgebase/m.md` reaches the memory under `/x/repo`, but git reports the real spelling, so the relative path came out as `../REPO/...` and the policy allowed the write. repo_relative() now falls back to a case-blind comparison. - check-knowledgebase.py accepted `---garbage` or `----` as the opening delimiter. Delimiters must now be whole `---` lines, as cairn writes them; keys still parse as cairn parses them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zhengyu123
left a comment
zhengyu123
left a comment
There was a problem hiding this comment.
Agentic review (Claude Code /code-review): 8 findings, ranked by severity. No crash risks. The first two are gaps in how well the guard protects .knowledgebase/; the rest are checker bugs, cleanup, and AGENTS.md rule issues.
Verification: check-knowledgebase.py passes on this tree (91 memories). utils/tests/test_knowledgebase_guard.sh passes: 45 tests, 0 failed. .knowledgebase/ content was not reviewed.
…(PROF-16469) Hook: - With a stale fork as origin, origin/main lacks the policy and the hook allowed every write. It now falls back to the checkout's policy (read as UTF-8; an undecodable file is skipped, not a crash) and allows only when neither has it. - The trust comment no longer claims the branch cannot weaken the guard: the rule comes from origin/main, but the hook and settings.json run from the checkout. The README says the same. - The three hooks that copy git_out and the trusted-ref loading now name each other; only this one falls back to the checkout. check-knowledgebase.py stays never stricter than cairn's contract: - trailing whitespace on the `---` lines is accepted, as cairn does; a leading BOM is still reported, since cairn finds no frontmatter then; - `deadend-*` ids with kind finding and quoted empty values pass, as in cairn; tests pin that parity; - files are read as UTF-8; shorter docstrings, the rule list moves to problems(). Tests: the checkout-policy fallback and its decode guard, a relative payload cwd (the checkout_of loop guard, timed out in Python, not GNU timeout), and T7m/p/q/r/s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 3 Pipeline jobs failed
Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: e5a05d0 | Docs | View more details | Give us feedback! |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for 16152b7: What to do next?
DetailsSince those jobs are not marked as being allowed to fail, the pipeline will most likely fail. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for c6aed08: What to do next?
DetailsSince those jobs are not marked as being allowed to fail, the pipeline will most likely fail. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
7c6e89b
into
main
What does this PR do?:
Enforces the
.knowledgebase/rule: only the cairn skills write it..claude/hooks/protect-knowledgebase.pydenies Edit, MultiEdit, Write and NotebookEdit into.knowledgebase/, in any checkout of this repo. It resolves relative,..,~, symlinked and not-yet-existing paths and matches case-blind. Like the gradlew hook, it loads its policy (utils/knowledgebase-write-policy.py) fromorigin/mainfalls back to the checkout's policy whenorigin/mainlacks it (a stale fork asorigin), and allows only when neither has it..pi/extensions/protect-knowledgebase.tscovers Pi'seditandwritetools, including Pi's@prefix and a session started in a subdirectory.utils/check-knowledgebase.pyruns inqualityChecksand checks every tracked memory for cairn's required frontmatter. It parses exactly as cairn does.pre-pr-reviewskill tells the reviewer to skip.knowledgebase/content; the review marker still covers the files.Motivation:
Part A of PROF-16469, decided as D3:
.knowledgebase/stays tracked; only cairn writes it; new memories travel with the change they came from, in its PR or a stacked one; reviews skip their content and check only the frontmatter.The old "Never review
.knowledgebase/" paragraph sat inside the cairn-managed block. cairn's session-start hook rewrites that block and dropped the paragraph, so committing the rewrite deleted the rule.Additional Notes:
.claude/settings.json, which affects every engineer's Claude Code session.sed -i,cp); the frontmatter check catches a hand-written memory instead..KnowledgeBase/is also blocked (case-blind match);id,title,kind,tags,source, slug and file name,dead-*kind), not the full contract;cairn maintenancevalidates the rest;settings.jsonrun from the checkout, so a branch can switch the guard off; it stops accidental agent writes, not a hostile branch;cairn memorizeaccepts (for exampletags: [], which cairn writes) passes CI;How to test the change?:
bash utils/tests/test_knowledgebase_guard.sh(A1)node:22-bookworm, Pi cases includedbash utils/tests/test_quality_check_scripts.sh(A2, T7 + T4 wiring)check-knowledgebase.pyon all 91 memories onmainorigin/mainat the candidate, asked to Edit a memorycairn memorizeof a new memory in the same clone_kb_agents_block()output/code-review(medium) on the full changeFor Datadog employees:
credentials of any kind, I've requested a security review (run the
dd:platform-security-reviewskill, or file a request via the PSEC review form).
bewairealso runs automatically on every PR.🤖 Generated with Claude Code