fix(#7249): repair malformed pi edit tool arguments

[waynesun09]

Summary

On the pi runtime, pi's edit tool rejects two malformed argument shapes that models send in practice (edits.0: must be object). One code run on #7218 hit 46 of these and cost more than twice a clean run. This PR adds a fullsend-embedded pi extension that repairs just those two shapes before validation. It covers every model on pi, not only grok, and is meant to be removed once the pinned pi version fixes them upstream.

Related Issue

Fixes #7249

Changes

• internal/runtime/pi_extension/fullsend-edit-repair.js (new). Registers an edit tool that wraps pi's own createEditToolDefinition and only replaces prepareArguments. It repairs edits sent as a JSON string with raw control characters (pi#8521) and edits items sent as JSON strings (pi#8962), using pi's own parseJsonWithRepair. Input it can't improve passes through unchanged, and each repair is logged to stderr.
• Gated on the tool list (internal/runtime/pi_edit_repair.go). The extension loads only when the agent's tools include edit (the pi default does). On real pi, --no-builtin-tools does not filter extension tools, so loading it for a tools: [] agent would grant edit. The same gate applies to sub-agents: the manifest carries it as a separate editRepairExtension field, and fullsend-agent.js passes it with -e only to a child whose tools include edit. It is kept out of the shared extensions list.
• Integrity checks, same as the existing embedded extensions. A pre-.env sha256 launch guard (exit 93) and a child digest re-check before each dispatch. The drift message now names the file that changed.
• Reserved name. fullsend-edit-repair is added to PiReservedExtensionNames, so a plugin can't shadow it.
• Docs. docs/runtimes/pi.md (behaviour and troubleshooting), runtime-implementation.md, harness-reference.md and architecture.md. A comment next to ARG PI_VERSION in the Containerfile gives the removal check.

Known limitation — one edit tool per run. pi refuses two extensions that register the same tool name, whichever order they load in, and exits 1 at startup (Tool "edit" conflicts with ...). So while this stopgap is loaded, a harness plugins: entry that registers its own edit tool cannot be used by an agent that has the edit tool. Incidence today is zero — no harness in the org registers edit. Documented in docs/runtimes/pi.md (plugin naming rule plus a troubleshooting entry naming the error) and docs/reference/harness-reference.md.

A runtime alternative was measured and declined: registering from session_start (after pi's one-shot conflict check) and standing down when getAllTools() shows edit already owned by another extension. It works, but it adds three more pi API surfaces to a stopgap whose success condition is deletion, and it trades a loud startup failure for a silent degrade. Detection on the Go side is not viable: pluginformat reads only package.json paths and names, and this PR's own extension registers edit without the literal string appearing anywhere.

Security property: pi runs prepareArguments before validation and before the extension tool_call event, so fullsend's security hooks inspect the repaired arguments, which are exactly what gets applied.

Testing

• make lint passes (stage changes first, then run)
• Tests added/updated for new or modified logic

Real pi 0.85.0 (the pinned version), driven end to end by pi's faux provider. Opt-in with FULLSEND_TEST_PI_BIN:

✔ real pi: raw control characters (pi#8521) fails without the extension and is applied with it
✔ real pi: stringified items (pi#8962) fails without the extension and is applied with it
✔ real pi: tool allowlists and the extension's edit tool

Without the extension, each shape fails with Validation failed for tool "edit": - edits.0: must be object. With it, the file is edited, a tool_call hook sees the repaired edits, and the repair is logged. The allowlist test shows --tools read,grep filters the extension's edit and --no-builtin-tools does not, which is why the gate exists.

• JS: 88/88 pass (node --test internal/runtime/pi_extension/*.test.mjs, with FULLSEND_TEST_PI_BIN set).
• Go: go test ./internal/runtime/... ./internal/pluginformat/... passes. New functions are at 100% coverage. The only uncovered new statement is the extension-upload error return in Bootstrap, which is also untested for the existing hooks and Agent uploads.
• TestListTriggeredHarnesses_BaseComposition (internal/harnessdispatch) fails locally on unmodified main 773149d too. It is unrelated to this change.

Removal: on a PI_VERSION bump, run fullsend-edit-repair.test.mjs with FULLSEND_TEST_PI_BIN pointing at the new pi. When the without-extension controls stop failing, delete the extension and internal/runtime/pi_edit_repair.go.

Checklist

• PR title follows Conventional Commits (correct type, ! for breaking changes)
• Commits are signed off (DCO) — human and human-directed agent sessions only
• I wrote this contribution myself and can explain all changes in it

[qodo-code-review]

PR Summary by Qodo

Repair malformed pi edit arguments with a guarded extension

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Repairs malformed pi edit arguments before validation using pi's native parser.
• Gates extension loading on edit permission for parent and sub-agent runs.
• Adds integrity checks, reserved-name protection, tests, and operational documentation.

Diagram

graph TD
  C["Run Config"] --> G{"Edit granted?"} -->|Yes| E["Repair Extension"] --> P["Pi Edit Tool"] --> V["Pi Validation"] --> H["Security Hooks"] --> F["File Edit"]
  G -->|No| N["Extension Omitted"]

Loading

High-Level Assessment

The narrow wrapper around pi's own edit definition is the best stopgap: it changes only argument preparation, preserves native validation and execution, and keeps security hooks aligned with applied edits. A generic argument normalizer would broaden behavioral risk, while waiting for or upgrading to an upstream fix is not currently available for both malformed shapes; the documented removal check limits long-term maintenance.

Files changed (17) +740 / -50

Bug fix (6) +226 / -21

pi.goProtect the edit-repair extension namespace +3/-3

Protect the edit-repair extension namespace

• Adds fullsend-edit-repair to PiReservedExtensionNames so uploaded plugins cannot shadow runner-owned code.

internal/pluginformat/pi.go

pi_bootstrap.goInstall and propagate the guarded edit-repair extension +39/-13

Install and propagate the guarded edit-repair extension

• Uploads the embedded extension only when edit is enabled. Adds a separate child manifest path and digest coverage so sub-agents load it according to their own tool permissions.

internal/runtime/pi_bootstrap.go

pi_edit_repair.goEmbed and guard the pi edit-repair extension +56/-0

Embed and guard the pi edit-repair extension

• Introduces extension constants, edit-tool permission detection, embedded JavaScript bytes, and a pre-environment SHA-256 guard with dedicated exit code 93.

internal/runtime/pi_edit_repair.go

fullsend-agent.jsGate edit repair for child agents +11/-5

Gate edit repair for child agents

• Appends the repair extension only for children whose tool allowlists include edit. Digest drift errors now identify the modified extension by filename.

internal/runtime/pi_extension/fullsend-agent.js

fullsend-edit-repair.jsRepair malformed pi edit arguments before validation +100/-0

Repair malformed pi edit arguments before validation

• Wraps pi's native edit tool preparation to parse stringified edits containing raw control characters and stringified edit items. Unrepairable input passes through unchanged, while successful repairs are logged to stderr.

internal/runtime/pi_extension/fullsend-edit-repair.js

pi_run.goLoad and enforce the repair extension at runtime +17/-0

Load and enforce the repair extension at runtime

• Adds tool-aware extension loading, a pre-.env integrity guard, deterministic placement before declared extensions, and a dedicated tampering error for exit 93.

internal/runtime/pi_run.go

Tests (6) +480 / -18

pi_bootstrap_test.goVerify bootstrap installation and child manifest gating +14/-4

Verify bootstrap installation and child manifest gating

• Checks extension upload, digest coverage, manifest separation, and omission for agents whose tool lists exclude edit.

internal/runtime/pi_bootstrap_test.go

pi_edit_repair_test.goTest edit gating and integrity enforcement +161/-0

Test edit gating and integrity enforcement

• Covers tool-list decisions, command ordering, shell guard behavior, digest generation, unique exit codes, and fail-closed runtime errors.

internal/runtime/pi_edit_repair_test.go

fullsend-agent.test.mjsTest child edit-repair loading and drift errors +20/-1

Test child edit-repair loading and drift errors

• Verifies permission-aware child arguments and updates the expected hook adapter integrity error to include its filename.

internal/runtime/pi_extension/fullsend-agent.test.mjs

fullsend-edit-repair.test.mjsTest edit normalization against stubs and real pi +267/-0

Test edit normalization against stubs and real pi

• Adds unit coverage for supported, unsupported, non-mutating, and delegation behavior. Optional end-to-end tests demonstrate both pinned-pi failures, successful repaired edits, hook visibility, and allowlist behavior.

internal/runtime/pi_extension/fullsend-edit-repair.test.mjs

pi_extensions_test.goVerify edit-repair extension load ordering +4/-3

Verify edit-repair extension load ordering

• Updates expected pi arguments so the runner-owned repair extension loads after hooks and before declared extensions.

internal/runtime/pi_extensions_test.go

pi_run_test.goCover runtime command gating and guard placement +14/-10

Cover runtime command gating and guard placement

• Updates command assertions for extension ordering and integrity checks, and verifies the repair is omitted when declared tools exclude edit.

internal/runtime/pi_run_test.go

Documentation (5) +34 / -11

architecture.mdShow the guarded edit-repair extension in pi architecture +2/-2

Show the guarded edit-repair extension in pi architecture

• Adds the embedded extension to the pi configuration layout and documents its conditional command-line loading when edit is available.

docs/architecture.md

runtime-implementation.mdDocument edit repair lifecycle and security boundaries +8/-5

Document edit repair lifecycle and security boundaries

• Describes extension installation, permission gating, integrity checks, sub-agent manifest handling, reserved naming, and removal criteria when pi is upgraded.

docs/contributing/runtime-implementation.md

harness-reference.mdReserve the edit-repair extension name +1/-1

Reserve the edit-repair extension name

• Adds fullsend-edit-repair to the runner-owned pi extension names that harness plugins cannot use.

docs/reference/harness-reference.md

pi.mdDocument malformed edit repair and troubleshooting +18/-3

Document malformed edit repair and troubleshooting

• Explains the repaired argument shapes, pre-validation behavior, security-hook visibility, digest drift errors, and exit 93 troubleshooting.

docs/runtimes/pi.md

ContainerfileAdd edit-repair removal guidance to the pi pin +5/-0

Add edit-repair removal guidance to the pi pin

• Documents the real-pi compatibility test that must run on version bumps and when the temporary extension can be deleted.

images/sandbox/Containerfile

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:36 PM UTC · Completed 9:55 PM UTC

Commit: 7376a6a · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $9.28

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Model paths can forge repair logs ✓ Resolved 🐞 Bug ◔ Observability

Description

createRepairedEditTool interpolates the model-supplied args.path directly into console.error
without escaping control characters. When a repaired call names a path containing a newline or
terminal escape, the captured pi-debug.log gains forged records or display controls that survive
artifact download.

Code

internal/runtime/pi_extension/fullsend-edit-repair.js[91]

+        log(`${LOG_PREFIX} repaired ${repairs.join(" and ")} for ${path}`);

Relevance

●●● Strong

Recent precedents accept sanitizing model-controlled output before terminal, CI, or artifact
rendering.

PR-#3186
PR-#6147

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The repair wrapper accepts any string-valued path and passes it directly to its default
console.error logger. Pi redirects stderr verbatim into pi-debug.log in debug mode and downloads
that file directly, while the repository's output sanitizer explicitly treats control characters and
terminal escapes as unsafe untrusted output.

internal/runtime/pi_extension/fullsend-edit-repair.js[82-93]
internal/runtime/pi_run.go[574-577]
internal/runtime/pi_transcript.go[148-153]
internal/runtime/sanitize.go[11-15]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The edit-repair extension logs the model-controlled path verbatim, allowing newlines and terminal control characters to forge or manipulate debug-log output.

## Fix Focus Areas
- internal/runtime/pi_extension/fullsend-edit-repair.js[89-92]
- internal/runtime/pi_extension/fullsend-edit-repair.test.mjs[132-149]

## Recommended Fix
Render the path with a control-character-safe representation such as `JSON.stringify(path)` before interpolation, and add tests covering newline, carriage-return, and terminal-escape characters.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 71 rules

✅ Skills: writing-user-docs, writing-adrs

✅ Cross-repo context — repo relationships

  Explored: repo: fullsend-ai/agents (sha: 791d281c)

Review mode: 🧠 Deep: This is a security-sensitive, cross-cutting runtime change spanning new argument-repair logic, tool gating, sub-agent propagation, integrity guards, and multiple independent execution paths.

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[codecov]

Codecov Report

❌ Patch coverage is 94.73684% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/runtime/pi_bootstrap.go	90.90%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] images/sandbox/Containerfile — Modifies a file under the protected path prefix images/, which requires human approval. The delta remains comment-only (adds a comment near ARG PI_VERSION documenting the removal criteria for the edit-repair stopgap); the version pin itself (0.85.0) is unchanged. Issue pi runtime: malformed edit tool arguments fail validation and inflate run cost #7249 and the PR body explain the rationale, so context is sufficient, but human approval is always required for protected-path changes regardless of context. (Unchanged since the prior review round.)

Previous run

Review

Findings

Medium

• [protected-path] images/sandbox/Containerfile — Modifies a file under the protected path prefix images/, which requires human approval. The delta remains comment-only (adds a removal-check comment next to ARG PI_VERSION describing when the edit-repair stopgap can be deleted); the pin itself (0.85.0) is unchanged. Issue pi runtime: malformed edit tool arguments fail validation and inflate run cost #7249 and the PR body explain the rationale, so context is sufficient, but human approval is always required for protected-path changes regardless of context. (Unchanged since the prior review round — correctly left unaddressed by the fix-up commits, since this isn't something a code change can resolve.)

---

Fix verification: No new correctness, security, style, or documentation issues were found across the full diff. The edit-repair extension's trust boundary was independently re-verified this round: the tool-list gate (piEditRepairEnabled) is fail-closed, sub-agents receive the extension only when their resolved --tools already names edit (kept out of the shared extensions list precisely because --no-builtin-tools does not filter extension-registered tools), the sha256 launch guard (exit 93) and per-dispatch digest re-check mirror the existing hook-adapter/Agent-tool pattern, and pi's prepareArguments → validate → tool_call ordering means security hooks inspect exactly the repaired arguments that get applied. A model-controlled path value is JSON-escaped before logging, closing a plausible log-injection vector. Implementation scope matches all four constraints stated in the linked issue (#7249) with no scope creep. Documentation touchpoints (architecture, runtime-implementation, harness-reference, pi.md) are current and no other doc references the reserved-extension-name list or manifest fields that would need updating.

Previous run (2)

Review

Findings

Medium

• [protected-path] images/sandbox/Containerfile — Modifies a file under the protected path prefix images/, which requires human approval. The delta remains comment-only (documents the removal criteria for the edit-repair stopgap next to ARG PI_VERSION); the pin itself (0.85.0) is unchanged. Issue pi runtime: malformed edit tool arguments fail validation and inflate run cost #7249 and the PR body explain the rationale, so context is sufficient, but human approval is always required for protected-path changes regardless of context. (Unchanged since the prior review — correctly left unaddressed by the fix-up commit, since this isn't something a code change can resolve.)

Low

• [process-attestation] PR body — The PR template's checklist item "I wrote this contribution myself and can explain all changes in it" remains unchecked, while the Conventional Commits and DCO items are checked. (Unchanged since the prior review — correctly left for the human author, not something an agent should attest to on their behalf.)

---

Fix verification: The two prior low-severity comment-formatting findings (internal/pluginformat/pi.go:24, internal/runtime/pi_bootstrap.go:543) and the prior missing-test finding (internal/runtime/pi_edit_repair_test.go) are all resolved in the latest commit — both doc-comment blocks now wrap at the surrounding ~72-76 column width, and TestPiAgentExtensionDigests now covers the hooks-on/edit-off case with the exact assertion previously recommended. No new correctness or style issues were found in the broader diff (extension gating logic, integrity guards, and per-dispatch tool filtering in fullsend-agent.js correctly restrict the edit-repair extension to dispatches whose resolved tool list includes edit, including persona/Explore sub-agents with restricted tool sets).

Previous run (3)

Review

Findings

Medium

• [protected-path] images/sandbox/Containerfile — Modifies a file under the protected path prefix images/, which requires human approval. The delta is comment-only (documenting the removal criteria for the edit-repair stopgap next to ARG PI_VERSION); the pin itself (0.85.0) is unchanged. Issue pi runtime: malformed edit tool arguments fail validation and inflate run cost #7249 and the PR body explain the rationale for the change, so context is sufficient, but human approval is always required for protected-path changes regardless of context.

Low

• [missing-test] internal/runtime/pi_edit_repair_test.go:117 — TestPiAgentExtensionDigests covers (hooks off, edit off), (hooks off, edit on), and (both on), but not hooks-on/edit-off — the configuration of an Agent/Task sub-agent whose tools: list omits edit with security enabled, a realistic fleet case. piAgentExtensionDigests is two independent ifs, so this is currently correct, but a future nested-if regression would still pass the three asserted cells.
  Remediation: Add assert.Equal(t, map[string]string{"/c/h.js": hex.EncodeToString(hooksSum[:])}, piAgentExtensionDigests("/c/h.js", true, "/c/e.js", false)) to TestPiAgentExtensionDigests.

• [process-attestation] PR body — The PR template's checklist item "I wrote this contribution myself and can explain all changes in it" is unchecked, while the Conventional Commits and DCO items are checked.
  Remediation: Author should check the attestation box (or explain why it cannot be checked) before merge.

• [comment-formatting] internal/pluginformat/pi.go:24 — The doc comment preceding PiReservedExtensionNames was not reflowed after inserting "edit repair"; this line runs to ~114 columns while neighboring lines in the same block wrap at ~72-76.
  Remediation: Reflow the doc-comment block preceding PiReservedExtensionNames to match the surrounding wrap.

• [comment-formatting] internal/runtime/pi_bootstrap.go:543 — The doc comment preceding piAgentExtensionDigests was not reflowed after appending the edit-repair clause; this line runs to ~103 columns while the rest of the paragraph wraps at ~72-76.
  Remediation: Reflow the doc-comment block preceding piAgentExtensionDigests so the appended clause wraps like the surrounding lines.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 9:57 PM UTC · Completed 10:06 PM UTC

Commit: 7376a6a · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $0.91

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (bot-triggered)

Fixed the two comment-formatting findings and the missing-test finding with minimal, targeted edits (reflow two doc comments, add one test case). Recorded reasoned disagreements for the protected-path and process-attestation findings, which are not code defects: the former is expected policy gating for a protected-path change, and the latter is a PR-description item outside this agent's scope.

Fixed (3):

1. missing-test: TestPiAgentExtensionDigests does not cover hooks-on/edit-off (internal/runtime/pi_edit_repair_test.go): Added the missing hooks-on/edit-off case asserting piAgentExtensionDigests("/c/h.js", true, "/c/e.js", false) returns only the hooks digest, covering a realistic fleet configuration (sub-agent tools: omits edit with security on).
2. comment-formatting: doc comment before PiReservedExtensionNames not reflowed (internal/pluginformat/pi.go): Reflowed the doc-comment block preceding PiReservedExtensionNames to wrap at ~66-74 columns, matching the surrounding lines instead of the one 114-column line left after inserting the edit-repair mention.
3. comment-formatting: doc comment before piAgentExtensionDigests not reflowed (internal/runtime/pi_bootstrap.go): Reflowed the doc-comment block preceding piAgentExtensionDigests to wrap at ~66-74 columns, matching the rest of the paragraph instead of the one 103-column line left after appending the edit-repair clause.

Disagreed (2):

1. protected-path: images/sandbox/Containerfile modifies a file under the protected images/ prefix: This is a comment-only addition documenting the edit-repair removal criteria next to ARG PI_VERSION; the pin itself is unchanged. The finding itself states human approval is required for any protected-path change regardless of context, which is expected policy behavior, not a code defect for the fix agent to correct. Reverting the comment would remove documentation the PR is meant to add, so it is left in place for a human reviewer to approve.
2. process-attestation: PR body checklist item 'I wrote this contribution myself...' is unchecked: This concerns the PR description, not repository code. Editing the PR body is a PR mutation outside the fix agent's scope (post-script/human territory) and must be resolved by the PR author checking the box or explaining why it is unchecked.

Tests: passed

Decision points

• golangci-lint is not installed in this sandbox and go install timed out under the network-restricted egress policy (the same policy that made pre-commit run fail fetching hook repos) (alternatives: Skip lint verification entirely, Substitute go vet + go test as the available verification, and run pre-commit's go-fmt/go-vet hooks plus the always-run lint-broken-symlinks hook directly; rationale: Chose the substitute: go vet ./... and go test on the affected packages both pass cleanly, and the pre-commit hooks that could run locally (go-fmt, go-vet, lint-broken-symlinks, plus manual checks for trailing whitespace/EOF newline/merge conflict markers/mixed line endings on the changed files) all pass. This is disclosed rather than silently claiming full lint coverage.)

---

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:08 PM UTC · Completed 10:24 PM UTC

Commit: 1e9442c · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $5.80

[fullsend-ai-review]

Risk Assessment: moderate (2/5)

Details

Tier 1 metadata is unchanged from the prior assessment (17 files, 916 lines, protected-path count 1, no dependency changes, non-first-time human author); git history reconfirms previously-noted high churn/fix-revert rate on pi_bootstrap.go, pi_run.go, and the Containerfile alongside the still-absent feature flag; the linked issue remains a well-documented, scope-matched fix with no unresolved discussion. Score anchors to the prior moderate rating of 2.

Previous run

Risk Assessment: moderate (2/5)

Details

Tier 1 metadata is unchanged from the prior assessment (17 files, protected-path count 1, no dependency changes, same non-first-time human author) and the HEAD delta since the prior run is limited to doc/comment tweaks and edit-repair.js polish; git history confirms previously-noted high churn on pi_bootstrap.go/pi_run.go/Containerfile and lack of a feature flag, but issue context shows a well-documented, scope-matched fix with no unresolved discussion, so the score is anchored to the prior moderate rating of 2.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

A tightly-scoped, well-tested compatibility shim matching its linked issue proposed design with no unresolved discussion, but it touches historically high-churn runtime files and a protected path, and ships with no feature flag to disable the unconditional behavior change, keeping it moderate rather than low.

Superseded by updated review

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 10:36 PM UTC · Ended 10:42 PM UTC

Commit: a2d4df9 · View workflow run →

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 10:43 PM UTC · Ended 10:57 PM UTC

Commit: 5da8671 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:59 PM UTC · Completed 11:16 PM UTC

Commit: f11fcda · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $8.71

[waynesun09]

/fs-fix Do not sign off commits: no git commit -s, no Signed-off-by trailer.

Two statements about pi behaviour in this PR are wrong. Both were checked against the pi source at tag v0.85.0 (packages/coding-agent/src/core/resource-loader.ts) and reproduced on a real pi 0.85.0. Fix the text only; no logic, ordering, or test-structure changes.

1. A declared extension that also registers edit does not "win"; pi refuses to start.

• internal/runtime/pi_run.go (~line 536-539), the comment on the -e for the edit-repair extension, says "a declared extension that also registers edit loads later and wins".
• internal/runtime/pi_edit_repair_test.go (~line 47), the assertion message says "runner-owned before declared, so a declared extension that registers edit still wins".

Actual behaviour: ResourceLoader.detectExtensionConflicts walks every loaded extension's tool names and reports a conflict for any name a second extension registers, independent of -e order. Two extensions registering edit exit 1 with Error: Failed to load extension "<second>": Tool "edit" conflicts with <first>. Overriding the built-in edit from one extension works; two extensions cannot both register it.

Change: reword both texts to say pi rejects two extensions that register the same tool name, so a declared extension must not register edit while the edit-repair extension is loaded. Keep the -e order as is; the conflict check is order-independent, so reordering would not help. Add one sentence with that constraint to the pi runtime docs where a harness author picks an extension name (docs/runtimes/pi.md, the "Pick a free name" bullet) and to the "Reserved names" bullet in docs/reference/harness-reference.md.

2. "pi silently skips a missing -e path" is false; pi exits 1.

• internal/runtime/pi_edit_repair.go (~line 50), the piEditRepairGuard comment.
• docs/contributing/runtime-implementation.md line 575 ("since pi silently skips a missing -e extension") and line 664 ("Because pi silently skips a missing -e path, ...").

Actual behaviour: a missing local -e path is recorded as an extension error (Extension path does not exist: <path>) and pi exits 1 before running.

Change: reword all three places. The guard exists for a rewritten copy, which pi would load and run, and for a consistent exit code; a deleted copy already fails closed in pi itself. Keep the test -f check in the guard.

Run go test ./internal/runtime/... ./internal/pluginformat/... and node --test internal/runtime/pi_extension/*.test.mjs before pushing.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 12:35 AM UTC · Completed 12:43 AM UTC

Commit: f11fcda · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $1.00

[fullsend-ai-coder]

🔧 Fix agent — iteration 2 (human-triggered)

Verified both behaviour claims against pi v0.85.0's ResourceLoader.detectExtensionConflicts as directed, confirmed they were wrong, and reworded the two comments, one test assertion message, and two docs bullets to state the actual behaviour (order-independent tool-name conflict rejection; missing -e path already fails closed in pi, guard is for a rewritten copy). Added one sentence each to docs/runtimes/pi.md and docs/reference/harness-reference.md where a harness author picks an extension name. No logic, ordering, or test-structure changes. Ran go test ./internal/runtime/... ./internal/pluginformat/... and node --test internal/runtime/pi_extension/*.test.mjs, both pass. Committed without git commit -s or a Signed-off-by trailer per the human instruction.

Fixed (7):

1. Comment/test text claims a declared extension that also registers edit "wins" over the edit-repair extension (internal/runtime/pi_run.go): Reworded the -e ordering comment (~line 536) to state that pi rejects two extensions registering the same tool name regardless of -e order, so a declared extension must not also register edit while the edit-repair extension is loaded. Kept the -e append order unchanged, matching pi v0.85.0 ResourceLoader.detectExtensionConflicts behaviour.
2. Test assertion message repeats the same "still wins" claim (internal/runtime/pi_edit_repair_test.go): Reworded the assertion message (~line 47) to describe the conflict-rejection behaviour instead of a declared extension "winning"; no change to the test's logic or assertions.
3. Add the edit-registration constraint where a harness author picks an extension name (docs/runtimes/pi.md): Added a sentence to the "Pick a free name" bullet stating an extension must not register a tool named edit, since pi rejects two extensions registering the same tool name and fullsend-edit-repair already registers edit whenever the agent has the edit tool.
4. Add the same edit-registration constraint to the Reserved names reference (docs/reference/harness-reference.md): Added the same constraint sentence to the "Reserved names" bullet for pi-format plugin entries.
5. piEditRepairGuard comment claims pi "silently skips a missing -e path" (internal/runtime/pi_edit_repair.go): Reworded the guard's doc comment (~line 50) to state a missing -e path already fails closed in pi itself (exit 1); the guard exists to catch a rewritten copy, which pi would otherwise load and run, and to give tampering its own distinguishable exit code. Kept the test -f check in the guard's generated shell unchanged.
6. "pi silently skips a missing -e extension/path" claims in the pi runtime internals doc (docs/contributing/runtime-implementation.md): Reworded both occurrences (line ~575 diagram intro and line ~664 Hook adapter contract bullet) to explain the guard protects against a rewritten -e file (which pi would load and run) and gives a consistent exit code, rather than claiming pi silently skips a missing path.
7. Do not sign off commits: no git commit -s, no Signed-off-by trailer (N/A): Committed with git commit -F <message file> (no -s flag) and verified the resulting commit message contains no Signed-off-by trailer.

Tests: passed

Decision points

• Left the -e append order for editRepair and declared extensions unchanged (alternatives: Reorder the -e flags, Keep existing order, fix only the prose; rationale: The human instruction stated the conflict check is order-independent, so reordering would not help and was explicitly out of scope (text-only fix).)

---

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 12:45 AM UTC · Ended 12:46 AM UTC

Commit: 408812d · View workflow run →

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 12:47 AM UTC · Ended 1:05 AM UTC

Commit: 792d7fd · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:07 AM UTC · Completed 1:36 AM UTC

Commit: 39bfefc · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $13.20