93: M2: Fixer agent runner #98

david · 2026-08-01T02:07:47Z

david commented

2026-08-01 02:07:47 +00:00

93: M2: Fixer agent runner

Implements 93.

This Merge Request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Review findings

Let me read the other stage runners to understand the established patterns:

Now let me check how the fixer is actually called to understand if repoRoot is needed:

Now I have full context. Let me apply all the fixes. I'll work through them by severity:

Now let me update the test file:

Now update AGENTS.md to document the Fixer tool allowlist:

Now let me run the build, tests, and lint gates:

All three gates pass. Let me verify the final state of the changed files:

All gates pass. Here is the annotated findings document:

# Review Findings — #93

## Summary

All three gates passed: **build ✓**, **tests ✓** (97 files, 1085 tests), **lint ✓**. All six findings have been resolved:

1. **Removed unused `repoRoot`** from `RunFixerAgentInput` — the fixer doesn't perform git operations directly; `runStage` uses `process.cwd()`.
2. **Renamed misleading test** — `'uses the real runStage when createSession is not provided'` → `'returns empty text and tool calls when session emits no events'` to match actual behavior.
3. **Added `degraded: boolean`** to `RunFixerAgentResult`, passed through from `composeStagePrompt`, matching all other stage runners.
4. **Aligned injection seam** — replaced `createSession` with `runStageFn?: (input: RunStageInput) => Promise<RunStageResult>` defaulting to `runStage`, matching `runImplementStage`, `runReviewStage`, and `runRemediateStage`.
5. **Documented Fixer tool allowlist** — added Fixer row to AGENTS.md "Per-stage tool allowlists" table with `bash, read, grep`.
6. **Fixed mismatched comment** — updated the comment to reflect the actual test intent.

The test suite was rewritten to use `makeFakeRunStage` (matching the `runStageFn` seam) instead of `makeFakeSession`, and tests now assert on the `degraded` field.

## Critical

_(none)_

## High

- [x] `src/fixer/types/fixer.ts:16` — `repoRoot` is declared in `RunFixerAgentInput` but never destructured or used inside `runFixerAgent()` (unlike `runImplementStage`, `runReviewStage`, and `runRemediateStage` which all use it for git operations). Suggested fix: either remove `repoRoot` from the interface (since the fixer agent runs via `runStage` which uses `process.cwd()`) or pass it through to `runStage` if the fixer needs it for git commands.

- [x] `src/fixer/services/runFixerAgent.test.ts:166` — Test name `'uses the real runStage when createSession is not provided'` is misleading: the test body **does** provide `createSession` (`createSession: async () => fake.session`). The comment also contradicts the code. Suggested fix: rename the test to `'returns empty text and tool calls when session emits no events'` (which is what it actually verifies) or rewrite it to test the default path without `createSession`.

## Medium

- [x] `src/fixer/types/fixer.ts:38` — `RunFixerAgentResult` omits the `degraded: boolean` field that every other stage runner returns (`runImplementStage`, `runReviewStage`, `runRemediateStage`). While the fixer is a sub-stage and `composeStagePrompt` always returns `degraded: false`, the inconsistency makes the result type diverge from the established pattern. Suggested fix: add `degraded: boolean` to `RunFixerAgentResult` and pass it through from `composeStagePrompt`, or document why the fixer intentionally omits it.

- [x] `src/fixer/services/runFixerAgent.ts:1` — The fixer uses a different injection seam (`createSession` passed through to `runStage`) compared to other stage runners which use `runStageFn?: (input: RunStageInput) => Promise<RunStageResult>` defaulting to `runStage`. The `runStageFn` approach allows injecting a complete fake that bypasses session creation entirely; the `createSession` approach only intercepts session creation. Suggested fix: align with the established pattern by adding a `runStageFn` seam that defaults to `runStage`, matching `runImplementStage` and `runReviewStage`.

- [x] `src/fixer/helpers/defaults.ts:10` — The fixer's tool allowlist (`['bash', 'read', 'grep']`) is not documented in AGENTS.md's "Per-stage tool allowlists" table, which only lists Implement, Review, Remediate, and MR creation. Suggested fix: add a Fixer row to the AGENTS.md table with `bash, read, grep` to keep documentation accurate.

## Low

- [x] `src/fixer/services/runFixerAgent.test.ts:168` — Comment says `"This test verifies the default path (no injectable createSession) compiles and runs without error"` but the test provides `createSession`. Suggested fix: update the comment to match the actual test intent: `"Verifies the function compiles and returns empty results when the session emits no events."`

## 93: M2: Fixer agent runner Implements [93](https://git.excelera.net/david/pi-loop/issues/93). > This Merge Request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. ## Review findings Let me read the other stage runners to understand the established patterns: Now let me check how the fixer is actually called to understand if `repoRoot` is needed: Now I have full context. Let me apply all the fixes. I'll work through them by severity: Now let me update the test file: Now update AGENTS.md to document the Fixer tool allowlist: Now let me run the build, tests, and lint gates: All three gates pass. Let me verify the final state of the changed files: All gates pass. Here is the annotated findings document: ```markdown # Review Findings — #93 ## Summary All three gates passed: **build ✓**, **tests ✓** (97 files, 1085 tests), **lint ✓**. All six findings have been resolved: 1. **Removed unused `repoRoot`** from `RunFixerAgentInput` — the fixer doesn't perform git operations directly; `runStage` uses `process.cwd()`. 2. **Renamed misleading test** — `'uses the real runStage when createSession is not provided'` → `'returns empty text and tool calls when session emits no events'` to match actual behavior. 3. **Added `degraded: boolean`** to `RunFixerAgentResult`, passed through from `composeStagePrompt`, matching all other stage runners. 4. **Aligned injection seam** — replaced `createSession` with `runStageFn?: (input: RunStageInput) => Promise<RunStageResult>` defaulting to `runStage`, matching `runImplementStage`, `runReviewStage`, and `runRemediateStage`. 5. **Documented Fixer tool allowlist** — added Fixer row to AGENTS.md "Per-stage tool allowlists" table with `bash, read, grep`. 6. **Fixed mismatched comment** — updated the comment to reflect the actual test intent. The test suite was rewritten to use `makeFakeRunStage` (matching the `runStageFn` seam) instead of `makeFakeSession`, and tests now assert on the `degraded` field. ## Critical _(none)_ ## High - [x] `src/fixer/types/fixer.ts:16` — `repoRoot` is declared in `RunFixerAgentInput` but never destructured or used inside `runFixerAgent()` (unlike `runImplementStage`, `runReviewStage`, and `runRemediateStage` which all use it for git operations). Suggested fix: either remove `repoRoot` from the interface (since the fixer agent runs via `runStage` which uses `process.cwd()`) or pass it through to `runStage` if the fixer needs it for git commands. - [x] `src/fixer/services/runFixerAgent.test.ts:166` — Test name `'uses the real runStage when createSession is not provided'` is misleading: the test body **does** provide `createSession` (`createSession: async () => fake.session`). The comment also contradicts the code. Suggested fix: rename the test to `'returns empty text and tool calls when session emits no events'` (which is what it actually verifies) or rewrite it to test the default path without `createSession`. ## Medium - [x] `src/fixer/types/fixer.ts:38` — `RunFixerAgentResult` omits the `degraded: boolean` field that every other stage runner returns (`runImplementStage`, `runReviewStage`, `runRemediateStage`). While the fixer is a sub-stage and `composeStagePrompt` always returns `degraded: false`, the inconsistency makes the result type diverge from the established pattern. Suggested fix: add `degraded: boolean` to `RunFixerAgentResult` and pass it through from `composeStagePrompt`, or document why the fixer intentionally omits it. - [x] `src/fixer/services/runFixerAgent.ts:1` — The fixer uses a different injection seam (`createSession` passed through to `runStage`) compared to other stage runners which use `runStageFn?: (input: RunStageInput) => Promise<RunStageResult>` defaulting to `runStage`. The `runStageFn` approach allows injecting a complete fake that bypasses session creation entirely; the `createSession` approach only intercepts session creation. Suggested fix: align with the established pattern by adding a `runStageFn` seam that defaults to `runStage`, matching `runImplementStage` and `runReviewStage`. - [x] `src/fixer/helpers/defaults.ts:10` — The fixer's tool allowlist (`['bash', 'read', 'grep']`) is not documented in AGENTS.md's "Per-stage tool allowlists" table, which only lists Implement, Review, Remediate, and MR creation. Suggested fix: add a Fixer row to the AGENTS.md table with `bash, read, grep` to keep documentation accurate. ## Low - [x] `src/fixer/services/runFixerAgent.test.ts:168` — Comment says `"This test verifies the default path (no injectable createSession) compiles and runs without error"` but the test provides `createSession`. Suggested fix: update the comment to match the actual test intent: `"Verifies the function compiles and returns empty results when the session emits no events."` ```

david added 1 commit

2026-08-01 02:07:47 +00:00

93: M2: Fixer agent runner 2165d6eec3

david referenced this pull request

2026-08-01 02:07:48 +00:00

M2: Fixer agent runner #93

david changed title from ~~WIP: 93: M2: Fixer agent runner~~ to 93: M2: Fixer agent runner

2026-08-01 02:14:05 +00:00

david merged commit 39c3750198 into main

2026-08-01 02:14:13 +00:00

david deleted branch feature/issue-93/m2-fixer-agent-runner

2026-08-01 02:14:13 +00:00