Skip to content

fix(agent): make the built-in agent runnable end-to-end (#626)#1549

Open
kriszyp wants to merge 4 commits into
mainfrom
kris/agent-runnable
Open

fix(agent): make the built-in agent runnable end-to-end (#626)#1549
kriszyp wants to merge 4 commits into
mainfrom
kris/agent-runnable

Conversation

@kriszyp

@kriszyp kriszyp commented Jul 1, 2026

Copy link
Copy Markdown
Member

Summary

The built-in agent scaffold (#839) is structurally complete but did not run against a live Harper instance. This PR fixes four defects — all found by driving the agent end-to-end with Gemini (via the OpenAI-compatible endpoint), after which it successfully self-built a Product component and served full REST CRUD.

Independent of #1545 (registry tools) and #1547 (inspector tools) — this fixes the merged scaffold itself. All three touch agent.ts, so expect a small merge overlap there.

Fixes

1. Session persistence — the critical one. Rows were written via raw primaryStore.put(id, row), producing a non-versioned (prefix-less) record. When such a classic-shared-structures record begins with msgpackr classic record-id #2 (byte 0x42 == 66), the RocksDB decode heuristic (RecordEncoder.ts:328-335) misreads it as a local-timestamp-prefixed record, strips 8 bytes, and the decode corrupts → Could not find typed structure N → null → "Unknown session". The whole agent was dead here.

Notably this is not typed-structure/structon divergence (#1163) and not a dep-version issue — randomAccessFields defaults false (classic msgpackr structures), and it reproduces same-run on rocksdb-js 2.3.0. It's purely the 0x42-vs-timestamp decode ambiguity that only bites prefix-less records.

Fix: write through the Resource-level .put(record) (transactional), which stages the version/local-timestamp metadata so the record carries a <32 prefix byte and decodes unambiguously — the same pattern DurableSubscriptionsSession uses. Reads stay on primaryStore.get (a versioned record's prefix decodes fine).

2. Frozen-record mutation. The store returns a frozen record, so the read-modify-write mutators threw Cannot assign to read only property 'status'. requireSession now returns a structuredClone. The cloning test-mock masked this — the mocks now return frozen records so it can't regress.

3. FS path scope. write_file/read_file demanded an absolute path the model can't know, so Gemini invented literal componentsRoot/… prefixes that fell outside scope. Relative paths now resolve against componentsRoot.

4. No system prompt. The agent had zero grounding. Added buildSystemPrompt(scopes) describing its componentsRoot/logDir, tool surface, and Harper-app shape — after which the model addresses paths correctly.

Verification

  • Live: Gemini-driven agent_prompt → self-built product_app/schema.graphql (@table @export) → PUT /Product/1 204 → GET /Product/1 returns the record. End-to-end CRUD on an agent-authored app.
  • Agent unit suite green; test mocks now return frozen records and exercise the Resource-level put.

🤖 Generated with Claude Code

@claude
fix(agent): make the built-in agent runnable end-to-end (#626)
aaf8b13 
The #839 scaffold was structurally complete but did not run against a live
Harper instance. Four fixes, all found via manual end-to-end testing (a
Gemini-driven agent that now self-builds a working REST app):

- Session persistence (the critical one): session rows were written via raw
  `primaryStore.put`, which produces a NON-versioned (prefix-less) record. When
  such a classic-shared-structures record begins with msgpackr classic record-id
  #2 (byte 0x42 == 66), the RocksDB decode heuristic (RecordEncoder.ts:328-335)
  misreads it as a local-timestamp-prefixed record, strips 8 bytes, and the
  decode corrupts → "Could not find typed structure N" → null → "Unknown
  session". Fix: write through the Resource-level `.put(record)`, whose
  transactional path stages the version/local-timestamp metadata so the record
  carries a `<32` prefix byte and decodes unambiguously (same pattern as
  DurableSubscriptionsSession). Not typed-struct divergence, not a dep version.

- Frozen-record mutation: the store returns a frozen record, so the mutators'
  in-place writes threw "Cannot assign to read only property 'status'".
  `requireSession` now returns a structuredClone. The cloning test-mock hid this;
  the mocks now return FROZEN records so it can't regress.

- FS path scope: `write_file`/`read_file` demanded an absolute path the model
  couldn't know, so it invented literal "componentsRoot/…" prefixes that fell
  outside scope. Relative paths now resolve against componentsRoot.

- No system prompt: the agent had zero grounding. Added buildSystemPrompt(scopes)
  describing its componentsRoot/logDir, tool surface, and Harper-app shape.

Independent of #1545 (registry) and #1547 (inspector); all three touch agent.ts,
so expect a small merge overlap there. Agent unit suite green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kriszyp kriszyp requested review from dawsontoth and heskew July 1, 2026 23:28
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Jul 1, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a system prompt for the Harper agent to provide grounding context on its capabilities and filesystem scopes. It also updates session persistence to use the transactional, resource-level put method instead of primaryStore.put to prevent data corruption, and returns cloned, mutable session objects to avoid errors when modifying frozen store records. Finally, it ensures relative paths in filesystem tools are resolved against the components root directory. The review feedback suggests pre-computing the system prompt during initialization rather than re-constructing it on every agent run, as the scopes remain static.

Comment thread agent/agent.ts Outdated
claude[bot]
claude Bot reviewed Jul 1, 2026
View reviewed changes
Comment thread agent/tools/fsTools.ts Outdated
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

dawsontoth
dawsontoth reviewed Jul 1, 2026
View reviewed changes
Comment thread agent/agent.ts Outdated
Comment thread agent/agent.ts Outdated
Comment thread agent/agent.ts Outdated
kriszyp and others added 3 commits July 4, 2026 05:57
@kriszyp @claude
feat(agent): consume MCP tool registry (Operations profile), RBAC-fil…
37a0438 
…tered (#626) (#1545)

* feat(agent): consume MCP tool registry (Operations profile), RBAC-filtered (#626)

Fold the unified MCP tool registry's Operations profile (#617) into the
built-in agent's toolset, fulfilling #626's tool-composition design: the agent
consumes the same registry the MCP server exposes, RBAC-filtered for its
configured user. Re-does the intent of the abandoned #893 against current main
(that branch was ~465 commits stale and used the pre-merge registry API).

- agent/registryTools.ts (new): ensureOperationsToolsRegistered() populates the
  Operations profile on the main thread (idempotent); composeRegistryTools()
  snapshots the profile, filters by visibleTo(agentUser), and adapts each
  ToolDef -> AgentTool (inputSchema->parameters, destructiveHint->approval gate,
  ToolResult->return value with isError->throw so the loop records a recoverable
  failure rather than aborting).
- agent/agent.ts: resolve the configured agent user via server.getUser (falls
  back to a super_user identity with a warning when unresolved) and thread the
  registry tools through both composeToolset calls.
- agent/toolset.ts: merge operator-only + registry tools; operator-only tools
  win on a name collision.
- unitTests/agent/registryTools.test.js: 10 tests over RBAC filtering, shape
  adaptation, destructive gating, result unwrapping, and the collision rule.
  No server boot / no LLM credits.

Enforcement note: visibleTo controls listing only; real RBAC runs per-call in
the operation handler via hdb_user set to the agent's identity.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): fail closed + per-call identity for registry tools (#626)

Cross-model review (Codex + Gemini) flagged two authorization defects in the
registry integration; both fixed at root:

- Fail-open super_user: resolveAgentUser previously fabricated a
  { super_user: true } identity whenever agent.user couldn't be resolved,
  silently escalating a misconfigured or transient restricted service account to
  admin. Now resolveAgentIdentity only falls back to super_user for the *default*
  hdb_agent bootstrap user (whose provisioning #626 defers); an explicitly
  configured user that won't resolve throws (fail closed), and the agent runs with
  only its operator-only tools until the operator fixes agent.user.

- Stale cached identity: the agent user was resolved once at startup and closed
  over in every tool handler, so a role revocation/change wasn't honored until
  restart. The enforcement identity is now resolved *per call* (composeRegistryTools
  takes a resolveIdentity thunk), mirroring how the MCP HTTP path re-auths per
  request. The startup snapshot is used only for visibleTo listing (not a boundary).

Adjudicated out: Gemini's "setConfig ignores agent.user changes" — set_agent_config
does not accept `user` (not in its patch keys), so it can't change at runtime.

Adds 2 unit tests: per-call re-resolution honors a live role change; a fail-closed
rejection propagates and the operation never dispatches. 51 agent unit tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(agent): V8 inspector (CDP) tools for debugging/profiling workers (#626) (#1547)

* feat(agent): V8 inspector (CDP) tools for debugging/profiling workers (#626)

Adds the operator-only inspector tool surface from the #626 design — the last
of the operator tools not yet built. The agent (main thread) attaches over the
Chrome DevTools Protocol to *worker* thread inspector ports to evaluate, set
breakpoints/logpoints, and CPU-profile a worker without stalling the thread it
runs on.

- agent/tools/inspectorTool.ts (new): buildInspectorTools(deps) → five tools:
  inspector_attach, inspector_evaluate, inspector_set_breakpoint,
  inspector_set_logpoint, inspector_profile_cpu. A minimal CDP client over `ws`
  (id-correlated request/response + event listeners), one reused connection per
  worker debug port. Deps (debug config + live worker count) are injected so the
  tool is unit-testable without a server boot.
- Safety envelope (resolvePort): requires threads_debug + threads_debug_startingPort,
  range-checks workerIndex against the live worker pool, and REJECTS workerIndex < 0
  (the main thread — a self-attach/breakpoint would deadlock the agent).
- Worker debug port = threads_debug_startingPort + workerIndex (mirrors
  server/threads/threadServer.js). Logpoints are non-pausing (breakpoint whose
  condition console.logs and returns false). CPU profiles are summarized to the
  hottest functions by self time so the observation stays small.
- evaluate + set_breakpoint are marked destructive (arbitrary in-worker code /
  can pause a live worker) → gated by the loop's approval flow.
- agent/toolset.ts + agent/agent.ts: compose inspector tools into the operator-only
  set; deps read from env (threads_debug*) and the live `workers` pool.
- unitTests/agent/inspectorTool.test.js: safety-envelope guards, summarizeProfile,
  and a LIVE CDP round-trip (opens a real inspector on an ephemeral port; drives
  attach + evaluate + profile). 60 agent unit tests green.

Stacked on #1545 (registry tools).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): harden inspector CDP client after cross-model review (#626)

Cross-model review (Gemini) flagged real lifecycle/concurrency hazards in the
CDP tooling; all addressed:

- openCdp never settled if the socket closed/aborted before 'open' → sessionFor
  hung the agent. Now a pre-open close/error/abort rejects the connect promise.
- Session-cache races: cache the connection *promise* (dedupes concurrent opens);
  evict by identity so a stale close from a replaced connection can't drop a live
  session; drop a failed connect so the next call retries.
- Breakpoints could wedge a worker (paused, no resume path). Every connection now
  registers a Debugger.paused handler that logs a stack snapshot and auto-resumes,
  so a hit is observable via the Harper log but never leaves the worker paused.
- Logpoint injection: logExpression is now JSON-encoded and eval'd (not spliced as
  raw code), so it can't break out of the wrapper to force a pause. set_logpoint
  is also marked destructive (it runs an expression in the worker on every hit),
  alongside evaluate and set_breakpoint.
- CDP calls now carry a per-call abort signal + 30s timeout, so an unresponsive
  worker can't hang the agent loop. The connect signal guards only the handshake
  (aborting one caller no longer tears down the shared connection).
- Strict workerIndex parsing: reject ""/null/false/[] instead of Number()-coercing
  them to worker 0.
- Profiler.disable on cleanup; resolvePort rejects a port past 65535.
- _closeInspectorSessions handles in-flight opens.

Also fixes a test-teardown deadlock: inspector.close() blocks until CDP clients
drop, so the live-round-trip after() hook only closes our client and lets --exit
tear down the inspector. Full agent unit suite: 61 passing, build clean.

Codex leg returned no structured findings.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): tolerate an already-active inspector in CDP round-trip test (#626)

The dev-mode install CI uses for unit tests sets threads.debug: true, so
threadServer opens the V8 inspector on the main process during the suite.
The live-CDP test's before hook then called inspector.open() unconditionally
and threw ERR_INSPECTOR_ALREADY_ACTIVATED. Reuse the live inspector when one
is already open; only open our own ephemeral port otherwise.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Kris Zyp <kris@harperdb.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Kris Zyp <kris@harperdb.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@kriszyp @claude
feat(agent): Harper best-practices grounding + agent.systemPromptAppe…
9dada5f 
…nd (#626) (#1560)

* feat(agent): consume MCP tool registry (Operations profile), RBAC-filtered (#626)

Fold the unified MCP tool registry's Operations profile (#617) into the
built-in agent's toolset, fulfilling #626's tool-composition design: the agent
consumes the same registry the MCP server exposes, RBAC-filtered for its
configured user. Re-does the intent of the abandoned #893 against current main
(that branch was ~465 commits stale and used the pre-merge registry API).

- agent/registryTools.ts (new): ensureOperationsToolsRegistered() populates the
  Operations profile on the main thread (idempotent); composeRegistryTools()
  snapshots the profile, filters by visibleTo(agentUser), and adapts each
  ToolDef -> AgentTool (inputSchema->parameters, destructiveHint->approval gate,
  ToolResult->return value with isError->throw so the loop records a recoverable
  failure rather than aborting).
- agent/agent.ts: resolve the configured agent user via server.getUser (falls
  back to a super_user identity with a warning when unresolved) and thread the
  registry tools through both composeToolset calls.
- agent/toolset.ts: merge operator-only + registry tools; operator-only tools
  win on a name collision.
- unitTests/agent/registryTools.test.js: 10 tests over RBAC filtering, shape
  adaptation, destructive gating, result unwrapping, and the collision rule.
  No server boot / no LLM credits.

Enforcement note: visibleTo controls listing only; real RBAC runs per-call in
the operation handler via hdb_user set to the agent's identity.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): fail closed + per-call identity for registry tools (#626)

Cross-model review (Codex + Gemini) flagged two authorization defects in the
registry integration; both fixed at root:

- Fail-open super_user: resolveAgentUser previously fabricated a
  { super_user: true } identity whenever agent.user couldn't be resolved,
  silently escalating a misconfigured or transient restricted service account to
  admin. Now resolveAgentIdentity only falls back to super_user for the *default*
  hdb_agent bootstrap user (whose provisioning #626 defers); an explicitly
  configured user that won't resolve throws (fail closed), and the agent runs with
  only its operator-only tools until the operator fixes agent.user.

- Stale cached identity: the agent user was resolved once at startup and closed
  over in every tool handler, so a role revocation/change wasn't honored until
  restart. The enforcement identity is now resolved *per call* (composeRegistryTools
  takes a resolveIdentity thunk), mirroring how the MCP HTTP path re-auths per
  request. The startup snapshot is used only for visibleTo listing (not a boundary).

Adjudicated out: Gemini's "setConfig ignores agent.user changes" — set_agent_config
does not accept `user` (not in its patch keys), so it can't change at runtime.

Adds 2 unit tests: per-call re-resolution honors a live role change; a fail-closed
rejection propagates and the operation never dispatches. 51 agent unit tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(agent): V8 inspector (CDP) tools for debugging/profiling workers (#626)

Adds the operator-only inspector tool surface from the #626 design — the last
of the operator tools not yet built. The agent (main thread) attaches over the
Chrome DevTools Protocol to *worker* thread inspector ports to evaluate, set
breakpoints/logpoints, and CPU-profile a worker without stalling the thread it
runs on.

- agent/tools/inspectorTool.ts (new): buildInspectorTools(deps) → five tools:
  inspector_attach, inspector_evaluate, inspector_set_breakpoint,
  inspector_set_logpoint, inspector_profile_cpu. A minimal CDP client over `ws`
  (id-correlated request/response + event listeners), one reused connection per
  worker debug port. Deps (debug config + live worker count) are injected so the
  tool is unit-testable without a server boot.
- Safety envelope (resolvePort): requires threads_debug + threads_debug_startingPort,
  range-checks workerIndex against the live worker pool, and REJECTS workerIndex < 0
  (the main thread — a self-attach/breakpoint would deadlock the agent).
- Worker debug port = threads_debug_startingPort + workerIndex (mirrors
  server/threads/threadServer.js). Logpoints are non-pausing (breakpoint whose
  condition console.logs and returns false). CPU profiles are summarized to the
  hottest functions by self time so the observation stays small.
- evaluate + set_breakpoint are marked destructive (arbitrary in-worker code /
  can pause a live worker) → gated by the loop's approval flow.
- agent/toolset.ts + agent/agent.ts: compose inspector tools into the operator-only
  set; deps read from env (threads_debug*) and the live `workers` pool.
- unitTests/agent/inspectorTool.test.js: safety-envelope guards, summarizeProfile,
  and a LIVE CDP round-trip (opens a real inspector on an ephemeral port; drives
  attach + evaluate + profile). 60 agent unit tests green.

Stacked on #1545 (registry tools).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): harden inspector CDP client after cross-model review (#626)

Cross-model review (Gemini) flagged real lifecycle/concurrency hazards in the
CDP tooling; all addressed:

- openCdp never settled if the socket closed/aborted before 'open' → sessionFor
  hung the agent. Now a pre-open close/error/abort rejects the connect promise.
- Session-cache races: cache the connection *promise* (dedupes concurrent opens);
  evict by identity so a stale close from a replaced connection can't drop a live
  session; drop a failed connect so the next call retries.
- Breakpoints could wedge a worker (paused, no resume path). Every connection now
  registers a Debugger.paused handler that logs a stack snapshot and auto-resumes,
  so a hit is observable via the Harper log but never leaves the worker paused.
- Logpoint injection: logExpression is now JSON-encoded and eval'd (not spliced as
  raw code), so it can't break out of the wrapper to force a pause. set_logpoint
  is also marked destructive (it runs an expression in the worker on every hit),
  alongside evaluate and set_breakpoint.
- CDP calls now carry a per-call abort signal + 30s timeout, so an unresponsive
  worker can't hang the agent loop. The connect signal guards only the handshake
  (aborting one caller no longer tears down the shared connection).
- Strict workerIndex parsing: reject ""/null/false/[] instead of Number()-coercing
  them to worker 0.
- Profiler.disable on cleanup; resolvePort rejects a port past 65535.
- _closeInspectorSessions handles in-flight opens.

Also fixes a test-teardown deadlock: inspector.close() blocks until CDP clients
drop, so the live-round-trip after() hook only closes our client and lets --exit
tear down the inspector. Full agent unit suite: 61 passing, build clean.

Codex leg returned no structured findings.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(agent): Harper best-practices grounding + agent.systemPromptAppend (#626)

Gives the built-in agent Harper conventions so it builds idiomatic apps, and
lets operators tune its persona/policy without a rebuild.

- Depends on @harperfast/skills and sources the `harper-best-practices` skill
  from it (versioned, no drift). Progressive disclosure, mirroring the skill's
  own design:
    * agent/bestPractices.ts (new): loadBestPracticesOverview() injects the
      SKILL.md overview (rule index + when-to-use, ~1.2k tokens) into the system
      prompt; buildBestPracticeTool() exposes a `harper_best_practice` tool that
      lists rules (no arg) or returns rules/<name>.md on demand — so the agent
      only spends context on the guidance relevant to the task. Both degrade to
      nothing if the package isn't resolvable (agent still runs). Rule arg is
      guarded against path traversal.
- agent.systemPromptAppend: operator text appended after the built-in grounding
  and the best-practices overview. Read from liveConfig each run, and accepted by
  set_agent_config, so it can be tuned on a running instance. Added to the config
  schema and AgentConfig.
- System prompt is now assembled: built-in grounding → best-practices overview →
  operator append.

Stacked on the inspector PR (kris/agent-inspector). Agent unit suite green
(incl. new bestPractices tests exercising the real skill package).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): tolerate a pre-existing inspector session in the live CDP test (#626)

node:inspector is a single process-wide agent. In CI, threads_debug defaults
to true (installer.ts DEV_MODE_CONFIG), and server/threads/threadServer.js
opens the main-thread inspector as a top-level side effect of merely being
imported (e.g. via DurableSubscriptionsSession.ts pulling in
whenComponentsLoaded) whenever that config is on. That leaves this process's
one inspector slot already occupied by the time the "live CDP round-trip"
suite's before() hook runs, so inspector.open() throws
ERR_INSPECTOR_ALREADY_ACTIVATED.

Check inspector.url() first and reuse whatever is already listening instead
of assuming the suite is the sole owner of the process's debug port.

Verified by reproducing the exact CI error locally (mocha --require a script
that opens the inspector before test files load), confirming the fix resolves
it, and running the suite in isolation, within unitTests/agent/**, and across
3 full unitTests/**/*test.*js runs (2985 passing, 0 failing each time).

* docs(#626): document @harperfast/skills dependency; drop node:assert/strict

* feat(agent): expose the built-in agent over MCP (curated tools) (#626)

Lets any MCP client drive the built-in agent (agent_prompt, get_agent_session,
list_agent_sessions, approve_agent_action, cancel_agent_run).

Why not just `mcp.operations.allow`: the generic MCP operations profile walks
OPERATION_FUNCTION_MAP once, BEFORE this component registers its operations, so
the agent ops are added to the map too late for the walk — allow-listing them has
no effect (confirmed live: profile builds at 14 tools, then the 6 agent ops
register; tools/list never shows them). So we register a curated agent tool set
directly into the registry AFTER the ops exist.

- agent/mcpTools.ts (new): registerAgentMcpTools(operations) adds curated tools
  (proper schemas/descriptions, destructive/read-only annotations) on the
  operations profile. visibleTo is super_user-only for listing; each handler
  dispatches to the operation's execute with the MCP caller as hdb_user, so the
  op's own super_user check + downstream RBAC still apply. set_agent_config is
  intentionally NOT exposed (operator/config action).
- agent/agent.ts: call it after registering the ops. Tools sit inertly in the
  registry unless the MCP HTTP surface is enabled.

Verified live over the MCP Streamable HTTP transport: an MCP client completed
initialize → tools/list (agent tools present) → tools/call agent_prompt →
get_agent_session, and the agent ran a tool and answered.

Stacked on the best-practices PR (kris/agent-bestpractices).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(agent): preserve non-standard error details in MCP tool error responses

Fall back to String(err) before the generic message so string/plain-object
thrown values aren't reduced to a useless generic message (#1561).

* fix(agent): read best-practices from @harperfast/skills exports, not the filesystem (#626)

Per review, @harperfast/skills exposes its skill content directly as module
exports — skillSummary (SKILL.md), ruleNames, and a rules name→markdown map —
so there's no need to resolve the package on disk and read files by hand.

- bestPractices.ts: import { ruleNames, rules, skillSummary } and serve from
  them. This removes the sync readdirSync/readFileSync in the tool handler (no
  event-loop blocking on the main thread), the require.resolve path walk (ESM
  require-undefined hazard), and the path-traversal regex — rule lookup is now a
  plain map access, so malformed/traversal names simply miss.
- agent.ts: tighten the systemPromptAppend guard to an explicit string check
  before trim().
- dependencies.md: rewrite the @harperfast/skills entry to reflect module-export
  consumption (no fs access, rule bodies resident in heap, hard runtime dep).
- test: traversal/malformed names now surface as "No such best-practice rule".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Kris Zyp <kris@harperdb.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@kriszyp @claude
fix(agent): scope-enum fs tools + precomputed prompt; address PR #1549 …
29ce3a4 
…review

- fs tools take a `root` scope enum (components/logs/config) + relative path,
  replacing the absolute-or-relative ambiguity and the "Do NOT prefix" prompt
  bandaid (dawsontoth). Writes stay confined to components; isInside enforcement
  unchanged. Absolute paths now rejected outright, tightening the surface.
- tool param descriptions no longer say "Absolute filesystem path" (claude).
- precompute the static grounding+best-practices system prompt once; only the
  operator systemPromptAppend is folded in per run (gemini).
- system prompt no longer enumerates individual tool names; the SDK tool schema
  is the source of truth (dawsontoth).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dawsontoth dawsontoth dawsontoth left review comments
@claude claude[bot] claude[bot] left review comments
@heskew heskew Awaiting requested review from heskew
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kriszyp @dawsontoth