Skip to content

docs(adr): an embedded agent that reads freely and writes through the gate - #32

Merged
protonspy merged 3 commits into
mainfrom
docs/embedded-agent
Aug 2, 2026
Merged

docs(adr): an embedded agent that reads freely and writes through the gate#32
protonspy merged 3 commits into
mainfrom
docs/embedded-agent

Conversation

@protonspy

@protonspy protonspy commented Aug 2, 2026

Copy link
Copy Markdown
Owner

adr:0019. One record, no code.

What it decides

adr:0003 decided the application does not call an LLM, and adr:0013 superseded that record while keeping the clause, calling it part of "the substance of that record". This narrows adr:0013: that one clause falls, the other two stand, and the second — write-time validation is what replaces the writer — now stands alone between a cheap model and the wiki.

It narrows rather than supersedes, following the precedent adr:0013 set with adr:0002 and adr:0018 set with adr:0013: the new record states the narrowing, the narrowed record is left untouched. adr:0003 is not edited either — it is already superseded, and knowledge-base.md says a superseded record is marked, never rewritten.

What changed is who installs the application. 0003's argument was competition with the harness the user already has, and for that user it holds entirely. What is left is the person who downloads a signed installer, has no harness, and finds a window that scaffolds, validates, records — and cannot write a page.

The split the record exists for is between reading and writing. Reading is unrestricted within the project — list, glob, grep, read, confined by assertWithin — because the methodology works in Claude Code by exploring, and the skills instruct the agent to use the project's own canonical term without ever naming a tool that would let it find out which term that is. Writing has one door: the validated one the editor already uses. adr:0003 named this shape itself — an embedded agent speaking the same tools, not a second writer with direct disk access.

Two claims the review killed

I ran code-review on it as a fact-check rather than a code review, and it was worth it — a decision record that misquotes the records it narrows is worse than no record.

  • plans/harness-portability.md was cited as shipped. It is a plan: 18 tasks, none ticked, and no code mentions Codex or opencode. The sentence it supported leaned on it. Corrected.
  • "The index gives only structural answers" was wrong. ow search exists and searches content. But runSearch returns { slug, title, matches } — which pages mention a term and how often, not the passage adr:0010 described. That turned out to be the better argument, and it is the one in the record now. adr:0010 is itself superseded by adr:0014, so it is cited as description rather than authority.

Five other claims checked out exactly, including the adr:0003 closing quote and the delivery.md reasoning about file-disjointness that the Consequences apply to subagents.

The costs, which are in the record

  • A well-formed and wrong page passes. The gate holds form and cannot hold meaning, and the models this door exists for are the cheap ones. This is the cost and nothing in the record mitigates it — the mitigation is the plan's job: distillation proposes and the user approves, conformance writes directly, every write carries its origin.
  • The Groq credential gets a second purpose, after adr:0013 made a point of there being exactly one secret.
  • The provider's model list is not a menu a user has the information to choose from.
  • The empty state that shipped in fix: the four dead controls, the empty wiki, and failures reported where they happen #29 stops being true.

Gates

No code changed, so tests and lint have nothing to say about this diff and were not run. npx @protonspy/scc validate is the applicable check and is clean. security-review was not run: there is no attacker-reachable path in a prose file, and the security question this record raises — an agent with write and shell access to the project — is a review for the implementation, not for the decision to constrain it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HBt5LRVJd5Z7nagJGRwvSC

Summary by CodeRabbit

  • Documentation
    • Added an architecture decision record describing how embedded agents can read project content and manage wiki pages safely.
    • Documents validated, logged, and undoable page creation, editing, renaming, and deletion.
    • Covers shared agent instructions, workflow capabilities, limitations, credential and model considerations, and future safety requirements.
    • Notes updates needed for empty-state messaging and safeguards around parallel agent activity.

protonspy and others added 2 commits August 1, 2026 23:55
…and writing through the gate

`adr:0003` decided the application does not call an LLM, and `adr:0013` kept
that clause and called it part of the substance of the record it superseded.
This narrows `adr:0013`: that one clause falls, the other two stand, and the
second — write-time validation is what replaces the writer — now stands alone
between a cheap model and the wiki.

What changed is who installs the application. 0003's argument was competition
with the harness the user already has, and for that user it still holds
entirely. `plans/harness-portability.md` served them further, plural. What is
left is the person who downloads a signed installer, has no harness, and finds
a window that scaffolds, validates, records and cannot write a page.

The split this record exists for is between reading and writing:

- reading is unrestricted within the project — list, glob, grep, read, every
  path confined by `assertWithin`, because the methodology works in Claude Code
  precisely by exploring, and an agent that cannot search for a term will coin
  a second name for a concept that already has one;
- writing has exactly one door, the validated one the editor already uses. No
  `write_file`, no `edit_file`, no shell. 0003 named this shape itself when it
  said the preserving path was an embedded agent speaking the same tools and
  **not a second writer with direct disk access** — which is the ergonomic
  default of every agent toolkit;
- the instructions are generated from the same skills and `CLAUDE.md` the
  external harness reads, so one convention has two consumers rather than two
  agents having one each.

The consequences are not comfortable and are recorded as such: a well-formed
and wrong page passes, the credential now has two purposes, the model list is
not a menu a user has the information to choose from, and the empty state that
shipped last week stops being true.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBt5LRVJd5Z7nagJGRwvSC
…ls already are

The review checked 0019 against the repository rather than against itself, and
two of its Context claims did not survive that.

- **`plans/harness-portability.md` is a plan, not a shipped state** — eighteen
  tasks, none ticked, and nothing in `packages/` or `apps/` mentions Codex or
  opencode. It was written as accomplished fact, and the sentence it supported
  ("the user with a harness is served, by several harnesses") leaned on that.
- **`ow search` exists and does search content**, so "the index gives only
  structural answers" was wrong. But `runSearch` returns
  `{ slug, title, matches }` — which pages mention a term and how many times,
  not the passage `adr:0010` described. That is the argument, and it is a
  better one: an agent told "three pages mention *cutover*" reads all three to
  learn how the term is used, where grep hands it the line. `adr:0010` is also
  superseded by `adr:0014`, so it is cited as description rather than as
  authority.

Two things the discussion since then settled, now in the record:

- **MCP's read-only rule does not transfer.** It exists because one resident
  process serves many projects to a caller it does not know. The embedded agent
  is one project, in process, started by the person who clicked. What applies
  instead is the store's own invariant — nothing enters `wiki/` unvalidated —
  which binds the human in the editor too, and takes nothing from the agent:
  `write_file` and `writePage` are the same act and only one is recoverable.
  Outside `wiki/`, a scratchpad is where a proposal lives before approval.
- **`scaffoldSkills` already emits the layout the agent toolkits expect** —
  `.claude/skills/<name>/SKILL.md` with `name` and `description`. A path to
  point at, not a format to convert. Recorded as luck rather than foresight,
  because nothing fails loudly the day the shapes diverge. And neither the
  skills nor `CLAUDE.md` name a search tool while both assume one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBt5LRVJd5Z7nagJGRwvSC
@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds ADR 0019. It defines an embedded agent with project-confined reads and validated wiki writes. It also standardizes generated harness instructions and records operational consequences.

Changes

Embedded agent architecture

Layer / File(s) Summary
Agent context and discovery
docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md
Documents the need for embedded agent support and the limits of lexical search results.
Read and write boundary
docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md
Permits project-confined reads and requires validated, logged, atomic, undoable writes for wiki/. Scratch writes remain outside wiki/.
Harness convention and consequences
docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md
Uses generated harness instructions and scaffolded skills for both agent types. Records consequences for valid-but-incorrect pages, credentials, model selection, writer drift, concurrency, and empty-state messaging.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • protonspy/open-wiki#5: ADR 0019 narrows ADR 0013 by allowing embedded LLM calls with validated wiki writes.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the addition of an embedded agent with unrestricted reads and gated writes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/embedded-agent

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md (1)

150-154: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift

Make stale generated instructions a blocking condition.

The ADR says the two writers must not drift, but identifies SKILLS_VERSION as only a signal. A stale scaffold can make embedded and external agents follow different conventions while structural validation still passes.

Require regeneration or refuse agent writes when the scaffold version is stale.

🤖 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
`@docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md`
around lines 150 - 154, Update the generated-instructions validation and agent
write path described in the ADR so a stale SKILLS_VERSION is a blocking
condition rather than merely a diagnostic signal. Require the scaffold to be
regenerated before writes proceed, or refuse the embedded agent write when its
version is stale, while preserving structural validation for current scaffolds.
🤖 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
`@docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md`:
- Around line 58-60: The ADR currently describes unrestricted project reads
without defining the resulting data-egress controls. Update the embedded-agent
policy section to specify user consent, sensitive-file handling, redaction,
retention, and behavior when the LLM provider fails, while preserving
assertWithin as the path-traversal boundary and explicitly distinguishing it
from provider disclosure control.
- Around line 161-162: Revise the parallelism guidance near the “Parallelism
across sources” statement to qualify that sources are safe to process in
parallel only when their concept scopes are disjoint. State that overlapping
concepts require a consolidation step before any writes, preserving the concern
about duplicate pages or canonical terms.
- Around line 69-80: The agent-tool write boundary must be deny-by-default:
update the hook handling around Write, Edit, MultiEdit, and Bash so only
validated wiki page operations and application-owned scratch storage are
permitted, while all other project paths and direct filesystem writes are
denied. Ensure configured MultiEdit is explicitly handled, then add coverage in
the existing hooks and gate-decision test suites for allowed scratch/page
operations and denied non-wiki or direct-write attempts.

---

Nitpick comments:
In
`@docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md`:
- Around line 150-154: Update the generated-instructions validation and agent
write path described in the ADR so a stale SKILLS_VERSION is a blocking
condition rather than merely a diagnostic signal. Require the scaffold to be
regenerated before writes proceed, or refuse the embedded agent write when its
version is stale, while preserving structural validation for current scaffolds.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 056af99b-939f-4152-8b2a-788ac968a885

📥 Commits

Reviewing files that changed from the base of the PR and between 26ad970 and 011b8ac.

📒 Files selected for processing (1)
  • docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md

Comment on lines +58 to +60
**Reading is unrestricted within the project.** The agent gets the harness set — list,
glob, grep, read — over the project directory, every path confined with `assertWithin`
the way `packages/mcp` already confines its own. This is a reversal of nothing: reads

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Document and enforce the LLM data-egress policy.

The embedded agent can read every path inside the project. The Groq credential also serves agent requests. When file content enters a prompt, that content leaves the project and reaches Groq.

Define consent, sensitive-file handling, redaction, retention, and provider failure behavior. assertWithin limits path traversal, but it does not limit disclosure to the LLM provider.

Also applies to: 138-143

🤖 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
`@docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md`
around lines 58 - 60, The ADR currently describes unrestricted project reads
without defining the resulting data-egress controls. Update the embedded-agent
policy section to specify user consent, sensitive-file handling, redaction,
retention, and behavior when the LLM provider fails, while preserving
assertWithin as the path-traversal boundary and explicitly distinguishing it
from provider disclosure control.

Comment on lines +69 to +80
**What does apply is the store's own invariant: nothing enters `wiki/` unvalidated.**
That is not distrust of the agent — the human typing in the editor goes through the same
door, and so does every hook. Frontmatter against the schema, wikilinks that resolve,
citations that point at a source and an instant that exist, the write atomic, the
operation logged with its origin and undoable. So the agent creates, edits, renames and
deletes pages through tools that do those things, and **no tool writes into `wiki/`
without passing through them**. Nothing is taken from the agent by this: `write_file` and
`writePage` are the same act, and only one of them is recoverable.

Outside `wiki/` the rule does not apply. A scratchpad — in memory, or under the
application's own temp — is where a proposal lives before anyone has approved it, and
that is a place the agent may write freely.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 5 --glob '!docs/**' \
  'assertWithin|writePage|write_file|edit_file|execute|wiki/' .

Repository: protonspy/open-wiki

Length of output: 50376


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- relevant source files ---'
fd -t f . packages/access/src packages/cli/src packages/cli/tests | sort | \
  rg '(paths|atomic-write|write|gate|hooks|commands/(edit|page|init)|main)\.(ts|tsx)$'

printf '%s\n' '--- hook implementation ---'
cat -n packages/cli/src/hooks.ts | sed -n '100,360p'

printf '%s\n' '--- gate command and dispatch references ---'
rg -n -C 8 'runPreToolUse|preWrite|writePage|atomicWrite|recordWrite|write_file|edit_file|execute|Bash|MultiEdit|Write|Edit' \
  packages/cli/src packages/cli/tests plugins apps/desktop/src

printf '%s\n' '--- path and write implementations ---'
cat -n packages/access/src/paths.ts
cat -n packages/access/src/write/atomic-write.ts
cat -n packages/access/src/write/record.ts 2>/dev/null || true
cat -n packages/access/src/write/log.ts

Repository: protonspy/open-wiki

Length of output: 50376


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- ADR contexts ---'
cat -n docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md | sed -n '55,95p'
cat -n docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md | sed -n '124,145p'

printf '%s\n' '--- direct filesystem write calls outside tests ---'
rg -n --glob '*.ts' --glob '*.tsx' --glob '!**/*.spec.ts' --glob '!**/*.test.ts' \
  '\b(writeFileSync|appendFileSync|renameSync|copyFileSync|rmSync|mkdirSync|createWriteStream|writeFile|appendFile|rename|copyFile|rm|mkdir)\s*\(' \
  packages/access/src packages/cli/src apps/desktop/src plugins

printf '%s\n' '--- configured agent tools and hook matchers ---'
cat -n plugins/open-wiki/hooks/hooks.json
rg -n -C 10 'HOOK_MATCHERS|PreToolUse|PostToolUse|write_file|edit_file|execute|conformance|directly' \
  packages/cli/src packages/cli/tests plugins docs/adr

Repository: protonspy/open-wiki

Length of output: 50377


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

# Read-only structural inventory of filesystem writes and hook decisions.
roots = [Path("packages/access/src"), Path("packages/cli/src"), Path("apps/desktop/src")]
write_re = re.compile(r'\b(?:writeFileSync|appendFileSync|renameSync|copyFileSync|rmSync|mkdirSync|createWriteStream)\s*\(')
for root in roots:
    for p in sorted(root.rglob("*.ts")):
        text = p.read_text()
        hits = [(i, line.strip()) for i, line in enumerate(text.splitlines(), 1) if write_re.search(line)]
        if hits:
            print(f"{p}:")
            for i, line in hits:
                print(f"  {i}: {line}")
PY

Repository: protonspy/open-wiki

Length of output: 3872


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- narrowed hook and gate files ---'
wc -l packages/cli/src/hooks.ts packages/cli/src/commands/gate.ts packages/cli/src/main.ts packages/access/src/paths.ts packages/access/src/write/atomic-write.ts
cat -n packages/cli/src/hooks.ts | sed -n '1,280p'
cat -n packages/cli/src/commands/gate.ts | sed -n '1,240p'
cat -n packages/access/src/paths.ts | sed -n '1,220p'
cat -n packages/access/src/write/atomic-write.ts | sed -n '1,150p'

printf '%s\n' '--- relevant tests only ---'
rg -n -C 6 'outside|project|\.claude|CLAUDE|README|notes|raw/|wiki/|Bash|Write|Edit|MultiEdit|execute|deny|allow' \
  packages/cli/tests/hooks.spec.ts packages/cli/tests/gate.spec.ts packages/access/tests apps/desktop/tests 2>/dev/null || true

printf '%s\n' '--- ADR references and conformance wording ---'
rg -n -C 6 'conformance|writes directly|write_file|edit_file|execute|scratchpad|outside `wiki/`|application.*temp|project path|filesystem' docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md docs/adr

Repository: protonspy/open-wiki

Length of output: 50378


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- exact boundary logic ---'
sed -n '1,260p' packages/cli/src/hooks.ts
sed -n '1,220p' packages/cli/src/commands/gate.ts
sed -n '1,180p' packages/access/src/paths.ts

printf '%s\n' '--- configured hook tools ---'
cat plugins/open-wiki/hooks/hooks.json
rg -n -C 8 'does not flag a write outside|outside the wiki|\.claude/settings|preWrite\(|detectShellWrite\(|Write|Edit|MultiEdit|Bash' packages/cli/tests/hooks.spec.ts packages/cli/tests/release.spec.ts

printf '%s\n' '--- ADR target passages ---'
sed -n '64,92p' docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md
sed -n '128,140p' docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md

Repository: protonspy/open-wiki

Length of output: 35516


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
for p in [
    Path("packages/cli/src/hooks.ts"),
    Path("packages/cli/tests/hooks.spec.ts"),
    Path("plugins/open-wiki/hooks/hooks.json"),
]:
    print(f"--- {p} ---")
    lines = p.read_text().splitlines()
    for n, line in enumerate(lines, 1):
        if any(term in line for term in (
            "function preWrite", "function runPreToolUse", "detectShellWrite",
            "isProtected", "wiki/", "notes/", "README.md", "permissionDecision",
            "Write", "Edit", "MultiEdit", "Bash", "matcher", "hook",
        )):
            lo, hi = max(1, n-2), min(len(lines), n+3)
            for i in range(lo, hi+1):
                print(f"{i}: {lines[i-1]}")
            print()
PY

Repository: protonspy/open-wiki

Length of output: 29933


Enforce a deny-by-default write boundary for agent tools.

packages/cli/src/hooks.ts allows non-wiki Write/Edit paths, only detects selected Bash writes to wiki/, and returns without handling configured MultiEdit. Restrict agent writes to validated page operations and application-owned scratch storage. Deny all other project paths and direct filesystem writes. Add coverage in packages/cli/tests/hooks.spec.ts and packages/access/tests/gate-decision.spec.ts.

🤖 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
`@docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md`
around lines 69 - 80, The agent-tool write boundary must be deny-by-default:
update the hook handling around Write, Edit, MultiEdit, and Bash so only
validated wiki page operations and application-owned scratch storage are
permitted, while all other project paths and direct filesystem writes are
denied. Ensure configured MultiEdit is explicitly handled, then add coverage in
the existing hooks and gate-decision test suites for allowed scratch/page
operations and denied non-wiki or direct-write attempts.

Comment on lines +161 to +162
Parallelism across *sources* is the safe split; across chunks of one source it needs a
consolidation step that is not optional.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Qualify the “safe split” for parallel work.

Different sources do not guarantee disjoint concepts. Two source-specific agents can still create the same page or canonical term, as described in Lines 159-160.

Limit parallelism to disjoint concept scopes and require consolidation before writes.

Proposed wording
-Parallelism across *sources* is the safe split; across chunks of one source it needs a
-consolidation step that is not optional.
+Parallelism across sources is safer only when the sources have disjoint concept scopes;
+every parallel batch still requires consolidation before it writes.
📝 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.

Suggested change
Parallelism across *sources* is the safe split; across chunks of one source it needs a
consolidation step that is not optional.
Parallelism across sources is safer only when the sources have disjoint concept scopes;
every parallel batch still requires consolidation before it writes.
🤖 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
`@docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md`
around lines 161 - 162, Revise the parallelism guidance near the “Parallelism
across sources” statement to qualify that sources are safe to process in
parallel only when their concept scopes are disjoint. State that overlapping
concepts require a consolidation step before any writes, preserving the concern
about duplicate pages or canonical terms.

… checked one

The record claimed a filesystem surface reaching the real disk is "the
ergonomic default of every such toolkit". Having read `deepagents@1.12.1` in
the package rather than in its documentation, that is not true and it is the
same overstatement the fact-check caught twice already: it defaults its
filesystem to an in-memory backend, hides `execute` unless the backend can
execute at all, and takes a tool allowlist whose own worked example is
`["read_file", "ls", "glob", "grep"]`.

What is true is narrower and more useful. The restriction is supported rather
than resisted — and **the guarding is what is not the default**: path
permissions allow when no rule matches, and confinement to a root directory is
opt-in per backend. A tool the application supplies is explicitly untouched by
the filesystem allowlist, which is what leaves the gate as the only way in.

So a new consequence, which is the operative one for whoever builds this: the
line has to be proved rather than configured. A configuration that looks right
and a configuration that refuses are different claims, and only the second is
testable — every constraint here needs a test that attempts the write and
watches it fail.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBt5LRVJd5Z7nagJGRwvSC
@protonspy

Copy link
Copy Markdown
Owner Author

Verified against the package, not the docs

f84b9e1 corrects a claim in the record and adds the consequence that came out of checking it. I installed deepagents@1.12.1 in an isolated scratch directory and read its type definitions rather than its documentation.

The record was overstating. It said a filesystem surface reaching the real disk is "the ergonomic default of every such toolkit". It is not:

  • the filesystem middleware's backend defaults to StateBackendin memory, not the disk;
  • execute is capability-gated and removed unless the resolved backend supports command execution, regardless of the allowlist;
  • the tool allowlist's own worked example in the package is tools: ["read_file", "ls", "glob", "grep"], captioned "Read/search-only filesystem access";
  • and — the clause that leaves the decision implementable — "User-provided non-filesystem tools are not affected by this allowlist." The application's own gated write tools survive the restriction that removes write_file.

What is not the default is the guarding, and that is the new consequence:

  • path permissions are permissive when no rule matches — "If no rule matches, access is allowed" — so a permission model failure fails open;
  • confinement to a root is opt-in: FilesystemBackend only disallows .. and ~ and keeps resolved paths inside the root when virtualMode: true;
  • permissions are not enforced on execute at all, and combining them with an execution-capable backend throws ConfigurationError rather than pretending.

Two things the discussion asked for turn out to be built in and are worth recording for whoever writes the plan: skills?: string[] exists in the JavaScript package (the quickstart page does not mention it), and memory?: string[] loads AGENTS.md-style files into the system prompt at startup. scaffoldSkills already writes .claude/skills/<name>/SKILL.md with name and description, which is the layout skills expects.

None of this changes the decision. It changes one sentence of its reasoning, and it adds the consequence that matters most to whoever implements it: the line has to be proved, not configured. A configuration that looks right and a configuration that refuses are different claims, and only the second is testable.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
`@docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md`:
- Around line 89-96: Update the ADR to define the embedded-agent security
boundary before implementation: limit built-in tools to read_file, ls, glob, and
grep; configure root confinement and fail-closed unmatched permissions; exclude
write_file, edit_file, delete, and execute; require application tools to enforce
assertWithin; and route page writes through gateWrite and writePage. Specify
integration coverage for valid page/temp writes and denied direct filesystem,
configuration, traversal, and execute attempts.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 28d25709-096d-46d7-91b4-70efcbf8c156

📥 Commits

Reviewing files that changed from the base of the PR and between 011b8ac and f84b9e1.

📒 Files selected for processing (1)
  • docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md

Comment on lines +89 to +96
That is a restriction the toolkits support rather than resist, and saying otherwise
would be the easy overstatement here: `deepagents@1.12.1` defaults its filesystem to an
in-memory backend, hides `execute` unless the backend can execute at all, and takes an
allowlist whose own worked example is `["read_file", "ls", "glob", "grep"]`. **What is
not the default is the guarding.** Its path permissions are permissive when no rule
matches, and confinement to a root directory is opt-in. So the constraint costs a
configuration, and holding it means proving the refusal rather than reading the option
back.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 6 \
  'deepagents|assertWithin|read_file|write_file|edit_file|execute|writePage|atomic|undo|allowlist|permission' .

Repository: protonspy/open-wiki

Length of output: 50376


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- ADR 0019 ---'
sed -n '1,190p' docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md

printf '%s\n' '--- candidate agent/config files ---'
git ls-files | rg -i 'deepagents|agent|mcp|claude|access|write|gate|package\.json|lock' | head -200

printf '%s\n' '--- focused access symbols ---'
rg -n -C 5 \
  'export .*assertWithin|function assertWithin|gateWrite|writePage|atomicWrite|read_file|write_file|edit_file|execute|allowlist|permission|deepagents' \
  packages/access packages/cli apps docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md \
  --glob '!**/node_modules/**' --glob '!**/dist/**' | head -500

Repository: protonspy/open-wiki

Length of output: 50375


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- deepagents references and dependency declarations ---'
rg -n -i 'deepagents|langchain|filesystem|backend|allowlist|assertWithin|embedded agent|embedded-agent' \
  --glob '!**/node_modules/**' --glob '!**/dist/**' . | head -400

printf '%s\n' '--- package manifests ---'
for f in package.json packages/access/package.json packages/cli/package.json packages/mcp/package.json apps/desktop/package.json; do
  if test -f "$f"; then
    echo "### $f"
    cat "$f"
  fi
done

printf '%s\n' '--- relevant gate and path implementation ---'
sed -n '1,180p' packages/access/src/gate/gate.ts
sed -n '1,130p' packages/access/src/paths.ts
sed -n '1,240p' packages/access/tests/gate-guard.spec.ts

printf '%s\n' '--- relevant application-tool tests ---'
sed -n '1,230p' packages/cli/tests/e2e.spec.ts
sed -n '1,180p' packages/cli/tests/gate.spec.ts

Repository: protonspy/open-wiki

Length of output: 49117


🌐 Web query:

deepagents 1.12.1 filesystem backend allowlist path permissions assertWithin execute read_file ls glob grep

💡 Result:

In deepagents 1.12.1, the FilesystemMiddleware supports a tools allowlist to explicitly control which built-in filesystem tools are exposed to an agent [1][2]. Filesystem Tool Allowlist You can restrict an agent's filesystem capabilities by passing a tools set to the FilesystemMiddleware constructor [1][2]. The allowlist governs the eight built-in tool names: ls, read_file, write_file, edit_file, delete, glob, grep, and execute [1][2]. Note that read_file is mandatory and must be included in any provided allowlist; omitting it will raise a ValueError [3][2]. Any tool name not in this list, or any custom tool added via the agent's primary tools argument, remains unaffected [1][3]. If you use a backend that does not support the execute tool, including execute in the allowlist is a no-op [2]. Filesystem Permissions and assertWithin Filesystem permissions provide path-based access control (e.g., allow/deny operations on specific paths) for the built-in filesystem tools [4][3]. These are defined using a list of FilesystemPermission rules passed to the agent during creation [4][3]. While there is no direct public method named assertWithin in the filesystem API, the system enforces path restrictions primarily through virtual mode [5][6] and FilesystemPermission middleware [4][7]. Key Security Considerations: 1. Permissions vs. Execution: Filesystem permissions do not apply to sandbox backends that support arbitrary shell command execution via the execute tool [4][3]. Attempting to use path-based permissions with a backend that supports execution will raise a NotImplementedError, unless all paths are scoped under known route prefixes [7]. 2. Virtual Mode: Using virtual_mode=True in your FilesystemBackend is recommended to anchor operations to a specific root directory and prevent path traversal attacks (e.g.,../, ~) [5][6]. 3. Middleware Override: If you replace the default FilesystemMiddleware, you must manually pass the backend and permissions to your custom instance, as they will not be automatically inherited from the parent create_deep_agent configuration [8][9].

Citations:


Define and test the embedded-agent boundary before implementation.

deepagents@1.12.1 limits built-in tools only; custom application tools are unaffected. Specify read-only built-ins (read_file, ls, glob, grep), root confinement, no write_file, edit_file, delete, or execute, and fail-closed unmatched permissions. Enforce assertWithin in application tools. Route page writes through gateWrite and writePage. Add integration tests for valid page and temp writes, and for denied direct filesystem, configuration, traversal, and execute attempts.

🤖 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
`@docs/adr/0019-an-embedded-agent-that-reads-freely-and-writes-through-the-gate.md`
around lines 89 - 96, Update the ADR to define the embedded-agent security
boundary before implementation: limit built-in tools to read_file, ls, glob, and
grep; configure root confinement and fail-closed unmatched permissions; exclude
write_file, edit_file, delete, and execute; require application tools to enforce
assertWithin; and route page writes through gateWrite and writePage. Specify
integration coverage for valid page/temp writes and denied direct filesystem,
configuration, traversal, and execute attempts.

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