# Implementation Plan: Enforce readOnly and tool policies at the Pi AgentSession boundary

Status: Active plan for issue #130

## Goal

Pass the final resolved tool-name set into Pi's `createAgentSession({ tools: names })` so the SDK-level session boundary enforces the computed allowlist. Block caller-supplied session options from widening a run-level or per-agent read-only fence.

## Verified Current Behavior

**Root cause:** `src/agent.ts` line 829 calls `createAgentSession({ customTools, ... })` but never sets the `tools` string-array allowlist. Pi SDK `CreateAgentSessionOptions.tools` (verified in `node_modules/@earendil-works/pi-coding-agent/dist/core/sdk.d.ts` line 44) is the session-level allowlist. When omitted, the SDK defaults to enabling all built-in tools (`read`, `bash`, `edit`, `write`), so the `customTools` filtering at line 694 is purely cosmetic — the session still activates every tool.

**Evidence chain:**

- `src/agent.ts:694-698`: `applyToolPolicy` correctly filters `customTools` via allowlist, denylist, and readOnly fence
- `src/agent.ts:829-850`: `createAgentSession` receives `customTools` but **no** `tools` parameter
- `src/agent.ts:842`: `...this.sessionOptions` spreads caller-provided session options — could carry `tools: ["read","bash","edit","write"]` that widens the fence
- `src/agent-registry.ts:195-218`: `applyToolPolicy` is pure and correct — the gap is downstream, not here
- Pi SDK `CreateAgentSessionOptions.tools?: string[]` (sdk.d.ts:44) — verified working on Pi 0.80.8 and 0.84.1 per issue probe
- `src/workflow.ts:1857-1858`: `toolNames` is computed correctly by workflow.ts and passed to `agentRunner.run()` — the policy resolution is sound, only the AgentSession boundary is missing

## Dependencies

- **Issue #129** (SDK compatibility): must land first — this plan targets the migrated `ModelRuntime`/`ModelRegistry` API from 0.80.8+
- No new SDK surface area needed — `tools?: string[]` is already part of `CreateAgentSessionOptions`

## Allowed Paths

1. **Compute tool names from `applyToolPolicy` result**: Extract the `name` field from the filtered `customTools` array and pass it as `tools` to `createAgentSession`
2. **Order of spread**: Place `tools: resolvedNames` AFTER `...this.sessionOptions` so it always overrides caller-supplied `tools`
3. **Preserve extension tools**: Do NOT pass a `tools` list that excludes genuine extension/context tools present in `customTools`. The resolved names include ALL tools in `customTools` after policy (including lean-ctx, web tools, structured_output)
4. **Handle empty allowlist**: When `applyToolPolicy` yields `[]` (deny-all fence), pass `tools: []` to createAgentSession
5. **`excludeTools` from `disallowedTools`**: If the resolved denylist survives the allowlist intersection (unlikely but possible), also pass `excludeTools` for defense in depth

## Denied / Out-of-Scope

- **Not changing `applyToolPolicy`**: it is pure and correct
- **Not changing workflow.ts policy resolution**: `intersectToolAllowlists`, `unionToolDenylists`, `effectiveReadOnly` are all sound
- **Not modifying agentType `.md` parsing**: `AgentDefinition.tools`/`disallowedTools` fields are wired correctly
- **Not adding new SDK features**: `tools` and `excludeTools` already exist on `CreateAgentSessionOptions`
- **Not addressing `/deep-research` command hardening** (tracked separately under issue #122)
- **Not changing skills allowlist**: that channel is independent (already has `skillsOverride` enforcement)

## Implementation Steps (dependency-ordered)

### Step 1: Extract tool names and pass to `createAgentSession`

**File:** `src/agent.ts`, `WorkflowAgent.run()`, lines 694–850

After `applyToolPolicy` produces the SDK `customTools`, build the final allowlist from two policy-authorized sources:

1. the filtered built-in/explicit/custom tool definitions; and
2. when no explicit run-level tool universe was supplied, extension tools from the exact resource loader passed to the session.

An explicit run-level `options.tools` remains a closed authority fence (saved read-only review workflows depend on that), so resource-loader extension tools are not unioned into that path. Apply the same allowlist, denylist, and read-only filters to discovered extension names before unioning them. Add `structured_output` after policy narrowing as today.

In the `createAgentSession` call, place both `customTools: filteredDefinitions` and `tools: resolvedNames` **after** `...this.sessionOptions`, along with the exact inspected resource loader:

```typescript
...this.sessionOptions,
customTools: filteredDefinitions,
resourceLoader: inspectedResourceLoader,
tools: resolvedNames,
...(resolvedModel ? { model: resolvedModel } : {}),
modelRuntime,
```

Do not pass a second, independently computed policy through `excludeTools`: `resolvedToolNames` is the canonical final intersection after allowlist, denylist, and read-only resolution. A redundant denylist risks divergence and is unnecessary when caller-supplied `sessionOptions.tools` is overwritten.

**Key invariant:** `customTools`, the inspected `resourceLoader`, and `tools` must follow `...this.sessionOptions`, so caller configuration cannot widen the resolved policy through `session.tools`, replace filtered definitions through `session.customTools`, or introduce an uninspected loader.

### Step 2: Preserve the complete resolved tool universe

**Files:** `src/agent.ts`, `src/workflow.ts`, manager/worktree construction as needed

The final session allowlist must be derived from the actual tool definitions available to that run after harness, agentType, explicit allow/deny, read-only, and structured-output resolution. Verify behavior for:

- default coding tools;
- injected web/evidence tools;
- extension/context tools exposed to subagent sessions;
- worktree-cwd rebuilt tools;
- `structured_output` added after policy filtering; and
- an empty allowlist (`tools: []`) as a deny-all fence.

If Pi only registers resource-loader extension tools after session construction, ensure the session-level allowlist includes the authorized names before refresh; do not accidentally hide all installed read/search tools by deriving names only from the four base definitions.

### Step 3: Add targeted regression test — real session tool enumeration

**File:** `tests/agent-session-tool-policy.test.ts` (new)

Test that uses `createAgentSession` (not mocks) to verify:

1. **readOnly fence**: a read-only agent receives NO `bash`, `edit`, `write` in the live session
2. **tool allowlist**: an explicit allowlist of `["read"]` restricts the session to only `read`
3. **disallow denylist**: `disallowedTools: ["bash"]` removes bash from the session
4. **Caller cannot widen**: `session: { tools: ["read", "bash", "edit", "write"] }` does NOT bypass a `readOnly: true` or `toolNames: ["read"]` policy (use a narrowly typed test cast if `WorkflowAgentOptions.session` intentionally omits `tools`)
5. **Extension tools preserved**: when a custom tool is in the allowlist, it survives in the session
6. **Empty allowlist = deny-all**: `toolNames: []` yields zero tools in the session

Use `createFauxCore` (post-#129) + minimal runtime wiring. Add a test-only inline Pi extension whose `before_agent_start` handler records `pi.getActiveTools()` and `pi.getAllTools()` from the exact session created by `WorkflowAgent.run()`. Successful text output alone is not authority evidence. Include a resource-loader extension tool to prove default runs preserve authorized extension tools while explicit run-level tool sets remain closed.

### Step 4: Extend the closest existing policy/runtime tests

Locate the actual existing harness/tool-gating test files before editing; do not assume filenames. Keep pure workflow-layer assertions, then add at least one real-session assertion proving the policy reaches `AgentSession`. Include a worktree-cwd case because `WorkflowAgent.run()` rebuilds coding tools when `options.cwd` differs.

### Step 5: Update `saved-commands.ts` readOnly tools

**File:** `src/saved-commands.ts`, line 83

`savedWorkflowExecutionPolicy` already passes `createReadOnlyTools(cwd)` for review workflows. After the fix, the `createAgentSession` boundary will enforce it correctly. Verify that review workflows (`/pr_adversarial_review`, etc.) remain functional.

## Targeted Tests

| Test | What it verifies | File |
|------|-----------------|------|
| readOnly fence at session boundary | No bash/edit/write in live session | `tests/agent-session-tool-policy.test.ts` |
| tool allowlist at session boundary | Only allowed tools active in session | `tests/agent-session-tool-policy.test.ts` |
| caller cannot widen via session options | `session: { tools: [...] }` overridden | `tests/agent-session-tool-policy.test.ts` |
| Extension tools preserved | Custom tools in allowlist survive | `tests/agent-session-tool-policy.test.ts` |
| Empty allowlist = deny-all | `toolNames: []` yields zero tools | `tests/agent-session-tool-policy.test.ts` |
| Review workflow still functional | `/pr_adversarial_review` uses read-only | `tests/saved-commands.test.ts` (extend) |
| Workflow tool gating still passes | `toolNames` reaches session correctly | `tests/workflow-tool-gating.test.ts` |

## Floor / Latest Compatibility

- **Floor**: Pi `>=0.80.8` — `CreateAgentSessionOptions.tools` verified working on 0.80.8 and 0.84.1
- **Latest**: Pi `0.84.1` (current peer dependency)
- No API surface change — `tools` is already in the SDK type, just unused
- No version branching needed

## Acceptance Checklist

| Criterion | Verification |
|-----------|-------------|
| Read-only reviewer receives no bash/edit/write in live AgentSession | New test + manual verification |
| Mutation-authorized worker still receives intended coding tools | Existing workflow tests |
| Explicit tool/agentType policies survive normal and worktree-cwd session construction | `workflow-tool-gating.test.ts` + new boundary tests |
| Existing review workflows remain operational using read/search tools | `saved-commands.test.ts` + manual `/pr_adversarial_review` smoke test |
| Full gate passes against floor (0.80.8) and current Pi (0.84.1) | CI dual-lane from #129 |
| Caller `session: { tools: [...] }` cannot widen readOnly fence | New test in `agent-session-tool-policy.test.ts` |

## Rollback / Recovery

- If a regression appears, stop/revert the candidate release while preserving the fail-closed requirement; do **not** ship a fallback to the known-bypassed `customTools`-only behavior.
- No persisted state or data migration is involved.
- If a future SDK changes `tools`, add an explicit compatibility adapter backed by floor/latest tests rather than weakening the authority fence.

## Deferred Work

- **Issue #122**: `/deep-research` command-specific hardening (separate authority boundary)
- **Issue #131**: Foundation workflow redesign (may refactor how tool policies compose)
- **Upstream Pi SDK**: Consider making `tools` required when `customTools` is provided (API contract improvement)
- **Runtime-level `tools` verification**: Add a post-session-check that asserts `session` tool set matches policy (defense-in-depth monitor)