fix(agent): bound retry loops + confabulation guards + idle-stop + Clear button + topbar model + lint fix - #4
Open
brainit-consulting wants to merge 18 commits into
Conversation
…alog perf When the configured local model does not support tool calls (e.g. gemma3:4b on Ollama), the orchestrator previously launched a Pi agent session that failed immediately, demoted the feature, and re-picked it within seconds — spamming the agent log and blocking real work. This change classifies such errors as permanent so the runner skips retries and the orchestrator skips the feature on subsequent picks for the rest of the process. An explicit "run queue" clears the per-process blocklist so users who fix their config can immediately retry. Also memoise the feature detail dialog so SSE log traffic during an active agent run no longer thrashes per-keystroke renders in the description and acceptance-criteria fields. Bonus: add aria-label to the dependency picker (jsx-a11y), and migrate the broken `next lint` script to a Next 16 flat ESLint config so the project's quality gate runs again. Verification evidence and PR scoping notes are in dev/bugtracker.md.
Adds the testing artifacts used to verify the BUG-001 / BUG-002 fixes end-to-end against a real local model: - docs/example-app-requirements.md — human spec for an example SaaS-ideas tracker app, broken into 10 atomic features sized for a small local model (each one fits in a single agent session). - docs/example-app-features.json — the same backlog as a machine-readable payload with stable string keys for dependency wiring. - scripts/load-example-features.mjs — POSTs the JSON into a project's backlog via the existing API (no direct DB writes), then wires dependencies via a follow-up call per feature. These are scaffolding for verifying harness changes — not part of the fix itself. Kept on this branch for reproducibility but should be left out of the upstream PR.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds permanent-error detection and blocklisting to orchestration and surfaces permanence/confabulation from the runner; memoizes and memo-wraps FeatureDetailDialog (with props type change and ARIA on dependency select); introduces a Completed-column bulk-clear action; adds ESLint flat config and lint script change; supplies example-app spec/backlog plus loader/reset CLIs; docs and stylesheet tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Agent Runner
participant Orch as Orchestrator
participant Feat as Feature Selector
participant API as Logger/Event Emitter
Runner->>API: emit "done" { outcome, permanent?, confabulation? }
API->>Orch: finalizeSession(..., options={ permanent, confabulation })
Orch->>Orch: update confabulationCounts or add id to permanentlyBlocked
Orch->>API: emit failure log (include guidance when permanent)
Orch->>Feat: request next ready feature (options.excludeIds = permanentlyBlocked)
Feat->>Feat: filter candidates by excludeIds, check dependencies, return next feature
Orch->>Runner: launch runner for selected feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (1)
1-17: 🛠️ Refactor suggestion | 🟠 MajorAdd an explicit Node.js 20+ runtime constraint.
package.jsonis missing anengines.nodefloor, so the runtime requirement is not enforced.Suggested patch
{ "name": "localforge", "version": "0.1.0", "private": true, + "engines": { + "node": ">=20" + }, "description": "LocalForge - long-running autonomous coding harness that uses local LLMs (LM Studio) to build applications on autopilot.", "scripts": {As per coding guidelines
package.json: "Require Node.js 20+ for runtime environment".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 1 - 17, Add an explicit Node.js runtime constraint to package.json by adding an "engines" field with "node": ">=20" (and optionally "npm" or "pnpm" min versions if desired) so the project enforces Node 20+; locate package.json (near the top-level keys like "name", "version", "scripts") and insert the engines object ensuring it is valid JSON and follows existing formatting.lib/agent/orchestrator.ts (1)
1149-1153:⚠️ Potential issue | 🟡 MinorReset the permanent blocklist in test cleanup.
__resetStateForTests()now leavespermanentlyBlockeddirty, so one test that simulates a permanent failure can leak into the next case and make the retry logic behave as if a feature were still blocked.Suggested fix
export function __resetStateForTests(): void { const state = getState(); state.running.clear(); state.events.removeAllListeners(); + state.permanentlyBlocked.clear(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/agent/orchestrator.ts` around lines 1149 - 1153, The test-reset helper __resetStateForTests() currently clears running and events but leaves state.permanentlyBlocked intact; update __resetStateForTests() to also clear the permanent blocklist by accessing getState() and calling the appropriate clear/reset on the permanentlyBlocked structure (e.g., state.permanentlyBlocked.clear() or reassigning an empty Set/Map) so tests cannot leak a permanently blocked entry into subsequent cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev/bugtracker.md`:
- Around line 9-10: The markdown line containing the section labels (BUG-*,
CHORE-*, ENH-*) is being parsed as emphasis because of the asterisks; update
that line to escape or code-format the labels so MD037 is satisfied — for
example replace BUG-*/CHORE-*/ENH-* with `BUG-*` / `CHORE-*` / `ENH-*` or escape
the asterisks (BUG-\* / CHORE-\* / ENH-\*) wherever those labels appear to
prevent markdown emphasis parsing.
In `@docs/example-app-requirements.md`:
- Around line 69-80: The two fenced code blocks (the ASCII layout diagram block
and the numbered flow diagram block) are missing a language tag; change their
opening backticks from ``` to ```text (i.e., add "text" after the triple
backticks) so markdownlint MD040 is satisfied for both the ASCII sidebar diagram
and the numbered flow diagram (also apply the same change to the additional
occurrences noted around lines 92-120).
In `@lib/agent/orchestrator.ts`:
- Around line 863-878: The permanent-failure log message currently tells users
the feature is "paused until next restart", which is inaccurate because
startAllAgents() clears getState().permanentlyBlocked on manual retry; update
the message built in the appendAgentLog call (around the permanent branch where
baseMsg is used) to instruct users to use the Start button or rerun
startAllAgents() / the manual retry action to clear the blocklist and resume the
feature instead of suggesting a full restart. Ensure references to
getState().permanentlyBlocked, appendAgentLog, and startAllAgents() are
accounted for so the message matches the actual retry path.
In `@scripts/load-example-features.mjs`:
- Around line 34-105: Preflight and/or make the import transactional so partial
imports can't occur: before creating remote rows, validate all dependsOn keys in
spec.features (iterate spec.features and check every f.dependsOn entry exists in
the spec key set and optionally run a cycle/topological check) and throw early
if any missing/cyclic references are found; alternatively wrap Phase 1 and Phase
2 in a try/catch and on failure use the keyToId map to delete any created
features via postJson(`${baseUrl}/api/projects/${projectId}/features/${id}`) (or
a provided delete endpoint) to roll back, ensuring postJson, spec.features,
keyToId, and the Phase 1/Phase 2 loops are updated accordingly.
---
Outside diff comments:
In `@lib/agent/orchestrator.ts`:
- Around line 1149-1153: The test-reset helper __resetStateForTests() currently
clears running and events but leaves state.permanentlyBlocked intact; update
__resetStateForTests() to also clear the permanent blocklist by accessing
getState() and calling the appropriate clear/reset on the permanentlyBlocked
structure (e.g., state.permanentlyBlocked.clear() or reassigning an empty
Set/Map) so tests cannot leak a permanently blocked entry into subsequent cases.
In `@package.json`:
- Around line 1-17: Add an explicit Node.js runtime constraint to package.json
by adding an "engines" field with "node": ">=20" (and optionally "npm" or "pnpm"
min versions if desired) so the project enforces Node 20+; locate package.json
(near the top-level keys like "name", "version", "scripts") and insert the
engines object ensuring it is valid JSON and follows existing formatting.
🪄 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
Run ID: c874f872-0c7c-4e0e-a110-ba8675d1b143
📒 Files selected for processing (10)
components/kanban/feature-detail-dialog.tsxdev/bugtracker.mddocs/example-app-features.jsondocs/example-app-requirements.mdeslint.config.mjslib/agent/orchestrator.tslib/features.tspackage.jsonscripts/agent-runner.mjsscripts/load-example-features.mjs
Comment on lines
+34
to
+105
| const jsonPath = path.resolve(__dirname, "..", "docs", "example-app-features.json"); | ||
| const raw = fs.readFileSync(jsonPath, "utf8"); | ||
| const spec = JSON.parse(raw); | ||
|
|
||
| if (!Array.isArray(spec.features) || spec.features.length === 0) { | ||
| console.error(`no features found in ${jsonPath}`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| async function postJson(url, body) { | ||
| const res = await fetch(url, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(body), | ||
| }); | ||
| const text = await res.text(); | ||
| let data; | ||
| try { | ||
| data = text.length > 0 ? JSON.parse(text) : {}; | ||
| } catch { | ||
| data = { raw: text }; | ||
| } | ||
| if (!res.ok) { | ||
| throw new Error(`POST ${url} → ${res.status}: ${data.error ?? text}`); | ||
| } | ||
| return data; | ||
| } | ||
|
|
||
| // Phase 1: create features. Map each spec key → real DB id so we can wire | ||
| // dependencies in phase 2. | ||
| const keyToId = new Map(); | ||
|
|
||
| console.log(`Creating ${spec.features.length} features in project ${projectId}...`); | ||
| for (const f of spec.features) { | ||
| const created = await postJson(`${baseUrl}/api/projects/${projectId}/features`, { | ||
| title: f.title, | ||
| description: f.description, | ||
| acceptanceCriteria: f.acceptanceCriteria, | ||
| category: f.category, | ||
| priority: f.priority, | ||
| }); | ||
| const id = created?.feature?.id; | ||
| if (typeof id !== "number") { | ||
| throw new Error(`unexpected response for ${f.key}: ${JSON.stringify(created)}`); | ||
| } | ||
| keyToId.set(f.key, id); | ||
| console.log(` + #${id} [${f.key}] ${f.title}`); | ||
| } | ||
|
|
||
| // Phase 2: wire dependencies. The dependencies endpoint expects the full | ||
| // list each time — we built each feature's full prereq set up front so | ||
| // one POST per feature is enough. | ||
| console.log("\nWiring dependencies..."); | ||
| let depCount = 0; | ||
| for (const f of spec.features) { | ||
| if (!Array.isArray(f.dependsOn) || f.dependsOn.length === 0) continue; | ||
| const featureId = keyToId.get(f.key); | ||
| const dependsOn = f.dependsOn.map((k) => { | ||
| const id = keyToId.get(k); | ||
| if (typeof id !== "number") { | ||
| throw new Error(`unknown dependency key "${k}" referenced by "${f.key}"`); | ||
| } | ||
| return id; | ||
| }); | ||
| await postJson(`${baseUrl}/api/features/${featureId}/dependencies`, { | ||
| dependsOn, | ||
| }); | ||
| depCount += dependsOn.length; | ||
| console.log( | ||
| ` • #${featureId} (${f.key}) depends on ${f.dependsOn.join(", ")} ` + | ||
| `→ [${dependsOn.join(", ")}]`, | ||
| ); |
There was a problem hiding this comment.
Validate the dependency graph before creating remote rows.
If phase 2 fails after phase 1 has already created some features, the loader leaves the project half-imported. Preflight all dependsOn references first, or make the import transactional, so this script is all-or-nothing.
Suggested fix
+const featureKeys = new Set(spec.features.map((feature) => feature.key));
+for (const feature of spec.features) {
+ for (const depKey of feature.dependsOn ?? []) {
+ if (!featureKeys.has(depKey)) {
+ throw new Error(`unknown dependency key "${depKey}" referenced by "${feature.key}"`);
+ }
+ }
+}
+
// Phase 1: create features. Map each spec key → real DB id so we can wire
// dependencies in phase 2.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/load-example-features.mjs` around lines 34 - 105, Preflight and/or
make the import transactional so partial imports can't occur: before creating
remote rows, validate all dependsOn keys in spec.features (iterate spec.features
and check every f.dependsOn entry exists in the spec key set and optionally run
a cycle/topological check) and throw early if any missing/cyclic references are
found; alternatively wrap Phase 1 and Phase 2 in a try/catch and on failure use
the keyToId map to delete any created features via
postJson(`${baseUrl}/api/projects/${projectId}/features/${id}`) (or a provided
delete endpoint) to roll back, ensuring postJson, spec.features, keyToId, and
the Phase 1/Phase 2 loops are updated accordingly.
…og new findings Mark BUG-002 (typing lag) and BUG-003 (a11y label) as VERIFIED with evidence from the 2026-04-29 second live run on DreamForgeIdeas using qwen2.5-coder:32b. The verification checklist now has all fix-related boxes ticked. Also captures three findings discovered during the verification session that are out of scope for this PR but worth tracking for future contributions: - BUG-004: clicking "run queue" from a non-kanban route silently no-ops. Hypothesis is a stale projectId in the click handler, but not yet triaged. - ENH-004: bulk-select and delete on the kanban — observed when needing to reset a 10-card backlog took ~30 clicks. - ENH-005: the "Won't fit — switch to a smaller model" warning is overly prescriptive. Verified during this session that an oversized model (qwen2.5-coder:32b on 8GB VRAM) runs successfully via Ollama's CPU spillover; the copy should explain the trade-off rather than imply the model can't be used.
Some local-model + provider combinations on Ollama (verified with qwen2.5-coder:32b on CPU spillover and llama3.2:3b) emit JSON-shaped tool calls as plain assistant text rather than as structured tool_use content blocks. Pi never executes them, the session ends with resultSubtype="success" and toolCalls=0, and the orchestrator marks the feature complete. Repeat 10 times across a backlog and you get a "Project Complete!" celebration screen for an empty project directory. This change adds a confabulation guard in runCodingAgentOnce(): if the session would otherwise be reported as ok but produced fewer than LOCALFORGE_MIN_TOOL_CALLS tool calls (default 1), force ok=false and set a descriptive errorMessage. The error explicitly names the likely cause (text-shaped tool calls), warns that no filesystem changes were made, and recommends alternative models known to be reliable on Ollama. The guard is configurable via LOCALFORGE_MIN_TOOL_CALLS=0 for users who genuinely have research-only / no-tool features. The new errorMessage intentionally does not match isPermanentError or isTransientError, so the runner gives up after the first attempt (retrying the same model on the same input is unlikely to behave differently) and the orchestrator demotes via the standard backlog priority bump rather than blocklisting the feature for the rest of the process. Verification evidence (from agent-runner-debug.log session 593) and full root-cause analysis are in dev/bugtracker.md ENH-001.
…ration) Resetting a kanban for re-runs (or undoing an AI-generated backlog) used to require opening every completed card individually, clicking Delete feature, and confirming — three clicks per card, ~30 clicks for a 10-feature project. This change adds a destructive Clear button to the Completed column header that wipes every card in the column after a single confirm prompt. Implementation: - New onClearCompleted prop on the shared KanbanColumn primitive (renders only when id === "completed" and displayCount > 0). - Matching prop on ForgeKanban's DroppableForgeColumn, with a new .col-clear class in globals.css to keep the workshop aesthetic and avoid inline styles per project rule. - Both KanbanBoard and ForgeKanban define a handleClearCompleted callback that fires N parallel DELETE /api/features/:id through the existing single-feature endpoint (no new bulk API needed). Failures surface via the existing dragError channel; the feature list is re-fetched regardless so the UI reflects whatever did get deleted. Why a smaller first iteration: the original ENH-004 sketch was a full checkbox-select-across-columns affordance. That redesigns the kanban interaction model and is appropriate as a separate PR. The column-scoped Clear button covers ~95% of the actual reset use case (it's the Completed column that piles up) and ships in a tight diff.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/forge/forge-kanban.tsx (1)
446-483:⚠️ Potential issue | 🟠 MajorSame re-entrant bulk-clear risk as in
components/kanban/kanban-board.tsx.Line 446-Line 483 can also execute concurrently on repeated clicks; apply the same in-flight guard (or shared helper) here to prevent duplicate DELETE batches.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/forge/forge-kanban.tsx` around lines 446 - 483, handleClearCompleted can run re-entrantly on repeated clicks causing duplicate DELETE batches; add an in-flight guard (e.g., a React.useRef<boolean> like isClearingRef) checked at the start of handleClearCompleted to return early if already running, set isClearingRef.current = true before the async work and reset it to false in a finally block, keep existing calls to setDragError and await load() unchanged, and reference the guard in the closure (no need to add it to the dependency array if using ref) so duplicate concurrent executions are prevented.dev/bugtracker.md (1)
9-10:⚠️ Potential issue | 🟡 MinorEscape/code-format wildcard labels to satisfy markdownlint (MD037).
Line 9 still uses raw
*in section labels, which is parsed as emphasis.Suggested patch
-Sections: BUG-* (defects), CHORE-* (project hygiene), ENH-* (proposed +Sections: `BUG-*` (defects), `CHORE-*` (project hygiene), `ENH-*` (proposed enhancements — not yet implemented).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/bugtracker.md` around lines 9 - 10, Escape or code-format the wildcard asterisks in the section labels to satisfy markdownlint MD037: replace the raw text "Sections: BUG-* (defects), CHORE-* (project hygiene), ENH-* (proposed enhancements — not yet implemented)." with either backtick-wrapped tokens (e.g. `BUG-*`, `CHORE-*`, `ENH-*`) or escape the asterisks (e.g. BUG-\*, CHORE-\*, ENH-\*) so Markdown no longer treats them as emphasis; update that exact string in the document.
🧹 Nitpick comments (2)
components/kanban/kanban-board.tsx (1)
261-266: Route bulk-clear feedback through the project notification stack.This new failure path only updates
dragError; for this user action, prefer Sonner/Web Notifications/Web Audio so feedback is consistent with notification standards.As per coding guidelines,
**/*.{tsx,ts}: Use Sonner for toast notifications, Web Notifications API, and Web Audio API for notifications.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/kanban/kanban-board.tsx` around lines 261 - 266, The current failure path only sets setDragError using firstFailure.reason; instead route this feedback through the app notification stack by calling the Sonner toast API (e.g., toast.error) with a clear message derived from firstFailure.reason (or the fallback string), and also trigger the Web Notifications API (check/request permission before showing a Notification) and play a short Web Audio cue via the Audio API to match the project's notification standards; keep calling setDragError if UI state still needs it but ensure the Sonner toast, Web Notification, and audio cue are invoked using the same error message so feedback is consistent across channels.app/globals.css (1)
446-457: Use Tailwind/shadcn styling instead of introducing.col-clearglobal CSS.Line 446-Line 457 adds a new bespoke global class for a TSX control. Prefer utility classes (or a shared shadcn button variant) to keep styling consistent and composable.
As per coding guidelines,
**/*.{tsx,css}: Use Tailwind CSS with shadcn/ui component library for styling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/globals.css` around lines 446 - 457, A new global CSS class ".col-clear" (and its :hover/:focus-visible rules) was added for a TSX control; replace this bespoke global styling with Tailwind utility classes or a shared shadcn/ui button variant instead: remove the ".col-clear" rules from globals.css and update the TSX that uses className="col-clear" to use equivalent Tailwind utilities (padding, font, gap, colors, border, rounded, hover/background, focus outline) or the existing shadcn Button/variant="destructive" pattern so styling stays consistent and composable across the app.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/kanban/kanban-board.tsx`:
- Around line 232-268: handleClearCompleted can be invoked re-entrantly causing
duplicate DELETEs; add an in-flight guard (e.g., a clearingRef or clearing
state) that is checked at the start of handleClearCompleted and set before
starting the async work, and cleared in a finally block so repeated clicks are
ignored while a clear is running; update any UI (button disabled) if present and
ensure you reference the guard inside handleClearCompleted and clear it after
await load(), while keeping setDragError and load usage intact.
---
Duplicate comments:
In `@components/forge/forge-kanban.tsx`:
- Around line 446-483: handleClearCompleted can run re-entrantly on repeated
clicks causing duplicate DELETE batches; add an in-flight guard (e.g., a
React.useRef<boolean> like isClearingRef) checked at the start of
handleClearCompleted to return early if already running, set
isClearingRef.current = true before the async work and reset it to false in a
finally block, keep existing calls to setDragError and await load() unchanged,
and reference the guard in the closure (no need to add it to the dependency
array if using ref) so duplicate concurrent executions are prevented.
In `@dev/bugtracker.md`:
- Around line 9-10: Escape or code-format the wildcard asterisks in the section
labels to satisfy markdownlint MD037: replace the raw text "Sections: BUG-*
(defects), CHORE-* (project hygiene), ENH-* (proposed enhancements — not yet
implemented)." with either backtick-wrapped tokens (e.g. `BUG-*`, `CHORE-*`,
`ENH-*`) or escape the asterisks (e.g. BUG-\*, CHORE-\*, ENH-\*) so Markdown no
longer treats them as emphasis; update that exact string in the document.
---
Nitpick comments:
In `@app/globals.css`:
- Around line 446-457: A new global CSS class ".col-clear" (and its
:hover/:focus-visible rules) was added for a TSX control; replace this bespoke
global styling with Tailwind utility classes or a shared shadcn/ui button
variant instead: remove the ".col-clear" rules from globals.css and update the
TSX that uses className="col-clear" to use equivalent Tailwind utilities
(padding, font, gap, colors, border, rounded, hover/background, focus outline)
or the existing shadcn Button/variant="destructive" pattern so styling stays
consistent and composable across the app.
In `@components/kanban/kanban-board.tsx`:
- Around line 261-266: The current failure path only sets setDragError using
firstFailure.reason; instead route this feedback through the app notification
stack by calling the Sonner toast API (e.g., toast.error) with a clear message
derived from firstFailure.reason (or the fallback string), and also trigger the
Web Notifications API (check/request permission before showing a Notification)
and play a short Web Audio cue via the Audio API to match the project's
notification standards; keep calling setDragError if UI state still needs it but
ensure the Sonner toast, Web Notification, and audio cue are invoked using the
same error message so feedback is consistent across channels.
🪄 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
Run ID: 21df24bf-7835-427c-9fbd-ade997d31cea
📒 Files selected for processing (5)
app/globals.csscomponents/forge/forge-kanban.tsxcomponents/kanban/kanban-board.tsxcomponents/kanban/kanban-column.tsxdev/bugtracker.md
Run-2 of the live verification (after the first ENH-001 iteration shipped)
exposed two gaps in the original tool-call-floor guard:
1. The model can game the floor by making one benign probing tool call
per session, then confabulating. toolCalls=1 slips past the
tool_calls < 1 check.
2. Even when each session is correctly rejected, the orchestrator's
auto-continue immediately re-picks the demoted feature (everything
else depends on it transitively), so the kanban falls into an
infinite demote-and-re-pick loop — same shape as BUG-001 but
triggered by confabulation instead of tool-incompatibility.
This change adds two more layers of defence:
Layer 2 — workspace-fingerprint check
Snapshots a lightweight fingerprint of the project working directory
(file count + sum of path-length + size + mtime, capped at 10000
entries, depth 6, skipping .pi/, node_modules/, .git/, .next/) before
the Pi prompt and after it returns. If the fingerprint is unchanged
AND LOCALFORGE_REQUIRE_FS_CHANGES is on (default), the session is
rejected with a clear "directory unchanged" errorMessage. Catches the
toolCalls=1 case the floor missed.
Layer 3 — orchestrator-side streak escalation
The runner now sets confabulation: true on its done event whenever
layer 1 or layer 2 trips. Orchestrator state gains a per-process
Map<featureId, count> alongside the existing permanentlyBlocked Set.
finalizeSession increments the counter on each confabulation failure
and resets it on any non-confabulation outcome. When the counter hits
CONFABULATION_BLOCK_THRESHOLD (2), the feature is added to
permanentlyBlocked — same exit path used for genuinely permanent
errors. startAllAgents clears both the blocklist and the counter map
(the user's explicit Run queue click is the signal that the
underlying config may have been fixed).
Verified live 2026-04-29 with llama3.2:latest on the standard
DreamForgeIdeas backlog. Debug log shows two CONFABULATION_COUNT
entries (count=1 then count=2) on feature #34, followed immediately by
MAYBE_CONTINUE_NO_MORE_FEATURES { filledSlots: 0 }. Loop stopped
cleanly; agents went idle. Pre-fix: same scenario looped indefinitely.
…change
ENH-006: surface the effective model + provider in the top bar so users
can see at a glance which model the orchestrator will actually use, and
which one is doing the work during a live run.
LocalForge has a global default model setting plus a per-project
override, and confusion about which is active is a real source of
mistakes — especially when retrying after switching models or when a
confabulation loop forces a model swap. The badge eliminates that
ambiguity. While a run is in flight the badge picks up the accent
colour so it's obvious which model is producing output.
Wiring:
- app-shell fetches GET /api/projects/:id/settings whenever the active
project changes or another part of the app bumps refreshTick. The
fetched value is keyed by projectId so a stale fetch from a previous
project never bleeds into a new one.
- The project-settings dialog dispatches a window
`localforge:settings-changed` event after a successful save.
app-shell listens for it and bumps refreshTick, so the badge updates
immediately without a page reload.
- TopBar gains an `activeModel: { model, provider } | null` prop and
renders a `.tb-model` pill between the workspace breadcrumb and the
flex spacer. Style is in globals.css to keep inline styles out of the
TSX (project rule). Pill is subtle when idle, tinted accent when a
run is live.
Two small fixes to the test scaffolding that surfaced during the
2026-04-29 live verification with gpt-oss:20b on Ollama (the only
locally-available model that returns structured tool_calls correctly):
1. The DreamForgeIdeas example spec hardcoded `H:\DreamForgeIdeas` as
the project path. The agent (correctly using its real tools) tried
to write there. The workspace guard correctly blocked every write
because the actual cwd is `H:\localforge\projects\dreamforgeideas-2`.
Changed the spec to instruct the agent to use the current working
directory and not to use absolute paths. Affects:
- docs/example-app-features.json (scaffold feature description)
- docs/example-app-requirements.md (Local path note + scaffold step)
2. New `scripts/reset-project-features.mjs` deletes every feature for a
given project regardless of status. The UI's per-column "Clear"
button (added in this PR for ENH-004) only handles the Completed
column — it can't clear in-progress or backlog cards in one go.
This script complements it by hitting the existing single-feature
DELETE endpoint for each feature in turn. Used between iterations
when the spec or features need a clean reload.
Usage: `node scripts/reset-project-features.mjs <projectId>`
…ker MD037 Addresses CodeRabbit review feedback on PR leonvanzyl#4: - handleClearCompleted in both KanbanBoard and ForgeKanban could fire multiple parallel DELETE batches if the user clicked the Clear button repeatedly before the first round of fetches resolved. Each batch targeted the same feature ids, so the second wave hit 404s on rows the first wave had already deleted, surfacing misleading errors. Add a `clearCompletedInFlightRef = React.useRef(false)` guard at the top of each component. handleClearCompleted returns early if the ref is already true; otherwise it sets it before the async work and resets it in a finally block so the ref stays consistent on success and on failure. - Wrap the BUG-*/CHORE-*/ENH-* glob patterns in dev/bugtracker.md as inline code so markdownlint's MD037 (no-space-in-emphasis) stops flagging them as broken emphasis markers. The two other CodeRabbit comments on this PR (use Tailwind/shadcn for .col-clear; route bulk-clear feedback through Sonner+WebNotifications +WebAudio) are scope-mismatches: - ForgeKanban consistently uses bespoke CSS classes from globals.css (col-head, col-title, col-body, card, add-card, board-section, search-box). The new .col-clear follows that convention. Migrating only .col-clear would create an inconsistent outlier inside ForgeKanban. - KanbanBoard already uses setDragError for its other failure paths (notably the drag-persist failure right above this one). Sonner is used in the project, but only for agent-run lifecycle toasts in components/agent/agent-notifications.tsx — not for kanban CRUD/drag failures. Migrating just this callsite would be inconsistent with the file's pattern. Both could be addressed in a separate, broader UI-consistency PR.
brainit-consulting
added a commit
to brainit-consulting/localforge
that referenced
this pull request
Apr 29, 2026
…ker MD037 Addresses CodeRabbit review feedback on PR leonvanzyl#4: - handleClearCompleted in both KanbanBoard and ForgeKanban could fire multiple parallel DELETE batches if the user clicked the Clear button repeatedly before the first round of fetches resolved. Each batch targeted the same feature ids, so the second wave hit 404s on rows the first wave had already deleted, surfacing misleading errors. Add a `clearCompletedInFlightRef = React.useRef(false)` guard at the top of each component. handleClearCompleted returns early if the ref is already true; otherwise it sets it before the async work and resets it in a finally block so the ref stays consistent on success and on failure. - Wrap the BUG-*/CHORE-*/ENH-* glob patterns in dev/bugtracker.md as inline code so markdownlint's MD037 (no-space-in-emphasis) stops flagging them as broken emphasis markers. The two other CodeRabbit comments on this PR (use Tailwind/shadcn for .col-clear; route bulk-clear feedback through Sonner+WebNotifications +WebAudio) are scope-mismatches: - ForgeKanban consistently uses bespoke CSS classes from globals.css (col-head, col-title, col-body, card, add-card, board-section, search-box). The new .col-clear follows that convention. Migrating only .col-clear would create an inconsistent outlier inside ForgeKanban. - KanbanBoard already uses setDragError for its other failure paths (notably the drag-persist failure right above this one). Sonner is used in the project, but only for agent-run lifecycle toasts in components/agent/agent-notifications.tsx — not for kanban CRUD/drag failures. Migrating just this callsite would be inconsistent with the file's pattern. Both could be addressed in a separate, broader UI-consistency PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eature Live verification with gpt-oss:20b on Ollama produced one legitimate feature completion (the scaffold step, where a single bash command does everything) and then a series of read-without-write confabulations on the multi-step features (db-setup, layout, etc.). Our fingerprint guard correctly caught and blocked them, so the harness stayed honest, but no real progress was made past the first feature. The model was technically capable (real tool calls, structured response format, no Ollama compat issues — those are confirmed working). The failure mode was instruction-following: it would Read package.json, search for hints, and conclude "I have analysed the codebase, the feature is implemented" without actually invoking Write/Edit/Bash. Two changes that target this directly: 1. scripts/agent-runner.mjs:buildCodingSystemPrompt now opens with an unmistakable rule: "a feature is NOT complete until you have actually WRITTEN or EDITED files on disk". Reads/lists/searches are explicitly framed as preparation, not implementation. A new self-check step requires the agent to count its Write/Edit/Bash calls before claiming success — if zero and the feature requires file changes, go back and implement. 2. docs/example-app-features.json now leads every feature with `ACTION:` and lists the exact files to create or edit, with content sketches inline. Each ends with a "the session is incomplete unless [specific files] exist on disk" reminder so the model has both a starting point AND a concrete completion signal. These are belt-and-suspenders fixes for smaller models that don't infer follow-through from descriptive prose. Stronger frontier models would likely complete the original concise spec; this version meets gpt-oss where it is.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/agent/orchestrator.ts (1)
205-226:⚠️ Potential issue | 🟡 MinorUpdate the test reset path for the new singleton state.
getState()now persistspermanentlyBlockedandconfabulationCounts, but__resetStateForTests()still only clearsrunningand listeners. That will make tests/dev helpers order-dependent once any case hits the new blocklist paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/agent/orchestrator.ts` around lines 205 - 226, Tests reset function __resetStateForTests() doesn't clear the newly persisted singleton fields, causing state leakage; update __resetStateForTests() to also reset g[GLOBAL_KEY].permanentlyBlocked (new Set()) and g[GLOBAL_KEY].confabulationCounts (new Map()) along with clearing running and resetting events/listener count so the GLOBAL_KEY state is fully back to an initial state compatible with getState().scripts/agent-runner.mjs (1)
1109-1143:⚠️ Potential issue | 🟠 MajorPreflight misconfigurations still bypass the new permanent-failure path.
This plumbing only works once
runCodingAgent()returns a flagged result. The earlier!baseUrlexit still emits a plain failuredoneevent, so a blank provider URL will keep demoting/re-picking instead of being blocklisted as a permanent configuration error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/agent-runner.mjs` around lines 1109 - 1143, The early exit that triggers on a missing/invalid provider URL (the !baseUrl path before runCodingAgent()) must be updated to return/emit the same permanent-failure shape used after runCodingAgent() so preflight misconfigs are blocklisted: instead of emitting a plain done failure, emit the same done event structure used in the post-agent block (type: "done", outcome: "failure", reason: a clear message about missing provider URL, permanent: true, confabulation: false), set doneEmitted = true, and call debugLog("DONE_EMITTED", { outcome: "failure", phase: "pi", permanent: true, confabulation: false }) and emitLog with an explanatory error; ensure this mirrors the handling around runCodingAgent() so the !baseUrl code path is treated as a permanent configuration error.
♻️ Duplicate comments (1)
docs/example-app-requirements.md (1)
72-83:⚠️ Potential issue | 🟡 MinorAdd language identifiers to both fenced diagram blocks (MD040).
Line 72 and Line 95 use plain triple backticks; markdownlint expects a language tag.
Suggested patch
-``` +```text ┌──────────────────────────────────────────────────────┐ │ Sidebar (~240px) │ Main content │ │ │ │ │ [+ Add New Idea] │ ┌─────────────────────┐ │ │ [View All Ideas] │ │ search input │ │ │ │ ├─────────────────────┤ │ │ │ │ ideas table │ │ │ │ │ name | status | ... │ │ │ │ └─────────────────────┘ │ └──────────────────────────────────────────────────────┘@@
-+text
- Scaffold Next.js
│
▼- Add SQLite + Drizzle
│
▼- Define ideas schema
│
▼- List/create ideas API ──────┐
│ │
▼ │- Update/delete ideas API │
│- Layout shell with sidebar ──┤
│
▼
7. Ideas table UI
│
▼
8. Add idea modal
│
▼
9. Inline edit + delete
│
▼
10. Sortable columns + searchAlso applies to: 95-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/example-app-requirements.md` around lines 72 - 83, The fenced code blocks containing the ASCII sidebar diagram (starting with "┌──────────────────────────────────────────────────────┐") and the numbered flow list (starting with "1. Scaffold Next.js") lack language identifiers; update both opening triple-backtick fences to include a language tag (e.g., ```text) so markdownlint MD040 is satisfied, leaving the block contents unchanged.
🧹 Nitpick comments (1)
docs/example-app-requirements.md (1)
206-206: Use “full-page reload” hyphenation.Line 206 reads better and more consistently as “full-page reload.”
Suggested patch
- a full page reload + a full-page reload🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/example-app-requirements.md` at line 206, Replace the instance of the phrase "a full page reload" with the hyphenated form "a full-page reload" wherever it appears (e.g., the current occurrence "a full page reload") to ensure consistent hyphenation and clearer phrasing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/example-app-requirements.md`:
- Around line 65-66: Update the timestamp descriptions for `createdAt` and
`updatedAt` to accurately reflect SQLite's behavior: either state that
CURRENT_TIMESTAMP returns the format "YYYY-MM-DD HH:MM:SS" (no 'T'/'Z') when
using `CURRENT_TIMESTAMP` for defaults, or replace the suggested default with
the `strftime('%Y-%m-%dT%H:%M:%fZ','now','utc')` expression and state that this
produces an ISO-8601 UTC timestamp; reference `createdAt`, `updatedAt`,
`CURRENT_TIMESTAMP`, and the `strftime` pattern so the reader can choose and
implement the correct option.
In `@lib/agent/orchestrator.ts`:
- Around line 1131-1145: The unconditional clears of
getState().permanentlyBlocked and getState().confabulationCounts must be scoped
to the current project instead of clearing the process-wide maps; replace the
unconditional getState().permanentlyBlocked.clear() and
getState().confabulationCounts.clear() calls in the Start handler with logic
that filters/removes only entries belonging to the current project (use the
current project identifier available in this context, e.g. projectId or
currentProject), update the debugLog calls to report only the entries actually
removed (e.g. Array.from(...).filter(...)), and ensure you only mutate the
specific keys/entries for that project rather than calling clear() on the entire
collection.
In `@scripts/agent-runner.mjs`:
- Around line 686-701: The isPermanentError function currently treats any
message containing "401" or "403" as permanent which is too broad; update
isPermanentError to use stricter matching for HTTP status codes (e.g., match
whole-word or token boundaries or common prefixes like "http", "status", or
"status code" before the digits) rather than simple substring includes: replace
the plain string patterns in permanentPatterns (or add a parallel regex list)
and change the return logic in isPermanentError to test those regexes (use
word-boundary regex like \b401\b/\b403\b or patterns like
/status\s*[:=]?\s*401/i or /http\/?\s*401/i) so only actual HTTP 401/403 errors
are considered permanent; keep the remaining literal patterns (e.g., "model not
found", "invalid api key") unchanged.
In `@scripts/reset-project-features.mjs`:
- Around line 33-46: The current fetch for project features treats any non-OK
response as a hard failure (process.exit(1)), but per the docs a missing project
should be a no-op; update the non-OK handling around listRes so that if
listRes.status === 404 you log that the project doesn't exist and call
process.exit(0) (idempotent no-op), while keeping process.exit(1) for other
non-OK statuses; make this change near the existing listRes check that precedes
reading listData/features for projectId.
---
Outside diff comments:
In `@lib/agent/orchestrator.ts`:
- Around line 205-226: Tests reset function __resetStateForTests() doesn't clear
the newly persisted singleton fields, causing state leakage; update
__resetStateForTests() to also reset g[GLOBAL_KEY].permanentlyBlocked (new
Set()) and g[GLOBAL_KEY].confabulationCounts (new Map()) along with clearing
running and resetting events/listener count so the GLOBAL_KEY state is fully
back to an initial state compatible with getState().
In `@scripts/agent-runner.mjs`:
- Around line 1109-1143: The early exit that triggers on a missing/invalid
provider URL (the !baseUrl path before runCodingAgent()) must be updated to
return/emit the same permanent-failure shape used after runCodingAgent() so
preflight misconfigs are blocklisted: instead of emitting a plain done failure,
emit the same done event structure used in the post-agent block (type: "done",
outcome: "failure", reason: a clear message about missing provider URL,
permanent: true, confabulation: false), set doneEmitted = true, and call
debugLog("DONE_EMITTED", { outcome: "failure", phase: "pi", permanent: true,
confabulation: false }) and emitLog with an explanatory error; ensure this
mirrors the handling around runCodingAgent() so the !baseUrl code path is
treated as a permanent configuration error.
---
Duplicate comments:
In `@docs/example-app-requirements.md`:
- Around line 72-83: The fenced code blocks containing the ASCII sidebar diagram
(starting with "┌──────────────────────────────────────────────────────┐") and
the numbered flow list (starting with "1. Scaffold Next.js") lack language
identifiers; update both opening triple-backtick fences to include a language
tag (e.g., ```text) so markdownlint MD040 is satisfied, leaving the block
contents unchanged.
---
Nitpick comments:
In `@docs/example-app-requirements.md`:
- Line 206: Replace the instance of the phrase "a full page reload" with the
hyphenated form "a full-page reload" wherever it appears (e.g., the current
occurrence "a full page reload") to ensure consistent hyphenation and clearer
phrasing.
🪄 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
Run ID: a16814c6-bb13-41ba-a499-f5ebc6709605
📒 Files selected for processing (12)
app/globals.csscomponents/app-shell/project-settings-dialog.tsxcomponents/forge/app-shell.tsxcomponents/forge/forge-kanban.tsxcomponents/forge/top-bar.tsxcomponents/kanban/kanban-board.tsxdev/bugtracker.mddocs/example-app-features.jsondocs/example-app-requirements.mdlib/agent/orchestrator.tsscripts/agent-runner.mjsscripts/reset-project-features.mjs
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/example-app-features.json
- app/globals.css
- components/kanban/kanban-board.tsx
Comment on lines
+65
to
+66
| | `createdAt` | text NOT NULL | ISO timestamp, default `CURRENT_TIMESTAMP` | | ||
| | `updatedAt` | text NOT NULL | ISO timestamp, refreshed on update | |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In SQLite, what exact format does CURRENT_TIMESTAMP return, and what default expression should be used to store ISO-8601 UTC timestamps with 'T' and 'Z'?
💡 Result:
CURRENT_TIMESTAMP in SQLite returns a UTC timestamp as a TEXT value in the format YYYY-MM-DD HH:MM:SS (seconds precision; no trailing “T” or “Z”). To store ISO-8601 UTC timestamps in the form YYYY-MM-DDTHH:MM:SS(.sss)Z, use a DEFAULT expression based on strftime with an explicit UTC conversion, e.g. (with milliseconds): DEFAULT ( strftime('%Y-%m-%dT%H:%M:%fZ','now','utc') ) (If your SQLite build does not include %f millisecond support in strftime, use a variant that builds fractional seconds separately; but the key point is: use datetime/strftime with the 'utc' modifier and include the literal 'T' and 'Z' in the format string.)
Citations:
- 1: https://sqlite.org/lang_datefunc.html
- 2: https://www.sqlite.org/datatype3.html
- 3: https://www.sqlite.org/lang_datefunc.html
🏁 Script executed:
fd example-app-requirements.mdRepository: leonvanzyl/localforge
Length of output: 98
🏁 Script executed:
sed -n '60,70p' docs/example-app-requirements.mdRepository: leonvanzyl/localforge
Length of output: 660
Align timestamp wording with actual SQLite default behavior.
Lines 65–66 claim "ISO timestamp" but CURRENT_TIMESTAMP returns YYYY-MM-DD HH:MM:SS format in SQLite, not ISO-8601 (which requires 'T' and 'Z'). This mismatch can lead to implementation drift. Either update the description to match CURRENT_TIMESTAMP's actual output, or use strftime('%Y-%m-%dT%H:%M:%fZ','now','utc') for true ISO-8601 UTC timestamps.
Suggested wording fix
-| `createdAt` | text NOT NULL | ISO timestamp, default `CURRENT_TIMESTAMP` |
-| `updatedAt` | text NOT NULL | ISO timestamp, refreshed on update |
+| `createdAt` | text NOT NULL | UTC timestamp, default `CURRENT_TIMESTAMP` |
+| `updatedAt` | text NOT NULL | UTC timestamp, refreshed on update |📝 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
| | `createdAt` | text NOT NULL | ISO timestamp, default `CURRENT_TIMESTAMP` | | |
| | `updatedAt` | text NOT NULL | ISO timestamp, refreshed on update | | |
| | `createdAt` | text NOT NULL | UTC timestamp, default `CURRENT_TIMESTAMP` | | |
| | `updatedAt` | text NOT NULL | UTC timestamp, refreshed on update | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/example-app-requirements.md` around lines 65 - 66, Update the timestamp
descriptions for `createdAt` and `updatedAt` to accurately reflect SQLite's
behavior: either state that CURRENT_TIMESTAMP returns the format "YYYY-MM-DD
HH:MM:SS" (no 'T'/'Z') when using `CURRENT_TIMESTAMP` for defaults, or replace
the suggested default with the `strftime('%Y-%m-%dT%H:%M:%fZ','now','utc')`
expression and state that this produces an ISO-8601 UTC timestamp; reference
`createdAt`, `updatedAt`, `CURRENT_TIMESTAMP`, and the `strftime` pattern so the
reader can choose and implement the correct option.
Comment on lines
+1131
to
+1145
| // An explicit Start (vs auto-continue) is the user's signal that they may | ||
| // have fixed the underlying config — clear any permanent-failure markers | ||
| // so previously-blocked features get another chance. | ||
| if (getState().permanentlyBlocked.size > 0) { | ||
| debugLog("START_ALL_AGENTS_CLEARING_BLOCKLIST", { | ||
| cleared: Array.from(getState().permanentlyBlocked), | ||
| }); | ||
| getState().permanentlyBlocked.clear(); | ||
| } | ||
| if (getState().confabulationCounts.size > 0) { | ||
| debugLog("START_ALL_AGENTS_CLEARING_CONFABULATION_COUNTS", { | ||
| cleared: Array.from(getState().confabulationCounts.entries()), | ||
| }); | ||
| getState().confabulationCounts.clear(); | ||
| } |
There was a problem hiding this comment.
Scope the unblock to the current project.
permanentlyBlocked and confabulationCounts live in the process-wide singleton, so this unconditional clear unblocks every project. If two projects are active, clicking Run queue in project A can resurrect a permanently failing feature in project B.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/agent/orchestrator.ts` around lines 1131 - 1145, The unconditional clears
of getState().permanentlyBlocked and getState().confabulationCounts must be
scoped to the current project instead of clearing the process-wide maps; replace
the unconditional getState().permanentlyBlocked.clear() and
getState().confabulationCounts.clear() calls in the Start handler with logic
that filters/removes only entries belonging to the current project (use the
current project identifier available in this context, e.g. projectId or
currentProject), update the debugLog calls to report only the entries actually
removed (e.g. Array.from(...).filter(...)), and ensure you only mutate the
specific keys/entries for that project rather than calling clear() on the entire
collection.
Comment on lines
+33
to
+46
| const listRes = await fetch(`${baseUrl}/api/projects/${projectId}/features`, { | ||
| cache: "no-store", | ||
| }); | ||
| if (!listRes.ok) { | ||
| console.error( | ||
| `GET ${baseUrl}/api/projects/${projectId}/features → ${listRes.status}`, | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| const listData = await listRes.json(); | ||
| const features = Array.isArray(listData?.features) ? listData.features : []; | ||
| if (features.length === 0) { | ||
| console.log(`Project ${projectId} has no features. Nothing to delete.`); | ||
| process.exit(0); |
There was a problem hiding this comment.
Treat a missing project as the documented no-op.
Lines 18-20 say nonexistent projects should be a no-op, but this branch exits 1 for every non-OK response. If the API returns 404 for an unknown project, callers will currently see a hard failure instead of an idempotent reset.
Proposed fix
const listRes = await fetch(`${baseUrl}/api/projects/${projectId}/features`, {
cache: "no-store",
});
if (!listRes.ok) {
+ if (listRes.status === 404) {
+ console.log(`Project ${projectId} not found. Nothing to delete.`);
+ process.exit(0);
+ }
console.error(
`GET ${baseUrl}/api/projects/${projectId}/features → ${listRes.status}`,
);
process.exit(1);
}📝 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
| const listRes = await fetch(`${baseUrl}/api/projects/${projectId}/features`, { | |
| cache: "no-store", | |
| }); | |
| if (!listRes.ok) { | |
| console.error( | |
| `GET ${baseUrl}/api/projects/${projectId}/features → ${listRes.status}`, | |
| ); | |
| process.exit(1); | |
| } | |
| const listData = await listRes.json(); | |
| const features = Array.isArray(listData?.features) ? listData.features : []; | |
| if (features.length === 0) { | |
| console.log(`Project ${projectId} has no features. Nothing to delete.`); | |
| process.exit(0); | |
| const listRes = await fetch(`${baseUrl}/api/projects/${projectId}/features`, { | |
| cache: "no-store", | |
| }); | |
| if (!listRes.ok) { | |
| if (listRes.status === 404) { | |
| console.log(`Project ${projectId} not found. Nothing to delete.`); | |
| process.exit(0); | |
| } | |
| console.error( | |
| `GET ${baseUrl}/api/projects/${projectId}/features → ${listRes.status}`, | |
| ); | |
| process.exit(1); | |
| } | |
| const listData = await listRes.json(); | |
| const features = Array.isArray(listData?.features) ? listData.features : []; | |
| if (features.length === 0) { | |
| console.log(`Project ${projectId} has no features. Nothing to delete.`); | |
| process.exit(0); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/reset-project-features.mjs` around lines 33 - 46, The current fetch
for project features treats any non-OK response as a hard failure
(process.exit(1)), but per the docs a missing project should be a no-op; update
the non-OK handling around listRes so that if listRes.status === 404 you log
that the project doesn't exist and call process.exit(0) (idempotent no-op),
while keeping process.exit(1) for other non-OK statuses; make this change near
the existing listRes check that precedes reading listData/features for
projectId.
The 2026-04-29 second iteration of the live verification with
gpt-oss:20b on Ollama (after the prescriptive-spec fix) surfaced a new
failure mode the existing guards do not catch.
Session 655 (feature #54 "Scaffold Next.js app") timeline:
- 18:39 session starts
- 18:40 agent runs `npx create-next-app@latest .` — real files appear
- 18:40-18:42 full Next.js scaffold lands on disk (~13 files)
- 18:42-19:09 agent makes 27 minutes of no-op tool calls: hallucinated
tool names like `feature_get_ready` and `assistant`,
repeated `Reading package.json`, `Listing files`, etc.
- 19:09 SESSION_TIMEOUT_MS (30 min) hits, watchdog kills the runner
- finalizeSession sees outcome=terminated and demotes the feature back
to backlog despite the work being legitimately done
This is distinct from confabulation:
- the fingerprint guard is happy (real files on disk)
- the tool-call floor is satisfied (real bash ran)
- BUT the agent never declared done, and the terminated outcome lost
credit for completed work
Proposed fix (documented for a future commit, not in this PR):
Add an idle-stop heuristic to scripts/agent-runner.mjs that periodically
re-fingerprints cwd, tracks lastFsChangeAt and lastBashAt, and triggers
a graceful "outcome: success" stop after LOCALFORGE_IDLE_STOP_MS
(default 5 min) of zero fs change AND zero bash activity. Marking the
outcome success preserves credit for completed work. Set the env var to
0 to disable.
Manual recovery for the live session that hit this: PATCHed
/api/features/54 to flip status to "completed" so the orchestrator
doesn't redo work that's already on disk.
Wraps the 2026-04-29 verification work with two user-facing additions:
README.md:
- Mention Ollama alongside LM Studio in prerequisites (the app supports
both; the README only mentioned LM Studio).
- Add `npm run lint` to the scripts table (we fixed this in CHORE-001;
users should know it exists).
- New "Model selection" section that explains, in plain terms, the
three classes of local models we observed:
1. tool-call broken (gemma3 etc.) — the harness pauses the feature
with guidance, no retry storm, but the user must switch models;
2. tool-call works, incremental edits unreliable (qwen2.5-coder:7b,
llama3.x) — works for one-shot scaffold features, fails on
multi-step edits; the workspace-fingerprint guard catches it
and demotes honestly rather than reporting fake progress;
3. tool-call works, multi-step works — 30B+ code models or
frontier-tier; gpt-oss:20b is close but not perfect.
Sets honest expectations rather than over-promising.
dev/bugtracker.md:
- New "2026-04-29 verification session — final summary" at the top
consolidating what shipped, what's verified live, and what the
remaining model-side limitation is. Concludes with the bottom line:
the harness no longer fakes completions, no longer infinite-loops,
no longer claims success on empty project directories. Whether a
given user can actually finish a backlog is downstream of their
model choice — a concern the README's new section addresses.
No code changes — these are documentation only.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
scripts/agent-runner.mjs (1)
701-717:⚠️ Potential issue | 🟠 MajorTighten the 401/403 permanent-error match before blocklisting the feature.
Matching bare
"401"/"403"substrings is still too broad here. Any unrelated message containing those digits can be treated as permanent and remove the feature from the picker for the rest of the process.Suggested patch
function isPermanentError(errorMessage) { if (!errorMessage) return false; const permanentPatterns = [ - "does not support tools", - "tool use is not supported", - "tools is not supported", - "model not found", - "unknown model", - "invalid api key", - "incorrect api key", - "unauthorized", - "401", - "403", + /does not support tools/i, + /tool use is not supported/i, + /tools is not supported/i, + /model not found/i, + /unknown model/i, + /invalid api key/i, + /incorrect api key/i, + /\bunauthorized\b/i, + /\b(?:http|status(?: code)?)\D{0,8}401\b/i, + /\b(?:http|status(?: code)?)\D{0,8}403\b/i, ]; - const lower = errorMessage.toLowerCase(); - return permanentPatterns.some((p) => lower.includes(p.toLowerCase())); + return permanentPatterns.some((pattern) => pattern.test(errorMessage)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/agent-runner.mjs` around lines 701 - 717, The isPermanentError function currently treats any errorMessage containing the substrings "401" or "403" as permanent, which is too broad; update the permanentPatterns check in isPermanentError to require tighter matches (e.g., match HTTP status tokens or words like "401", "401 Unauthorized", "HTTP 401", and similarly for 403) by using word-boundary/regex checks or explicit token matching instead of simple substring includes; modify the logic in isPermanentError to test those patterns with a regex or by splitting the message into tokens so only true HTTP status indications (e.g., /\b401\b/, /\b403\b/, /\b401 Unauthorized\b/, /\b403 Forbidden\b/) trigger a permanent error, leaving other patterns (like "invalid api key", "unknown model") unchanged.
🧹 Nitpick comments (1)
scripts/agent-runner.mjs (1)
1139-1143: Centralize the recommended-model guidance instead of hardcoding it again here.This message already diverges from the confabulation-guard copy in the same file, so adjacent failure modes will send users to different “known-good” model lists. A shared constant would keep the guidance consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/agent-runner.mjs` around lines 1139 - 1143, Create a shared constant for the "recommended models" guidance and replace the hardcoded message here and the other confabulation-guard message with that constant to keep guidance consistent; e.g., define a clear identifier like RECOMMENDED_TOOL_MODELS_GUIDANCE near the top of scripts/agent-runner.mjs (or in an imported constants module) and use it in the emitLog call (and the other occurrence) so both failure modes reference the exact same string while preserving the existing log level and formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev/bugtracker.md`:
- Around line 83-85: The statement claiming "llama3.2:latest successfully
cleared the in-process blocklist and re-picked feature `#4`" conflicts with later
notes that local models didn't finish DreamForgeIdeas and that "llama3.2:latest"
emitted plain‑text tool calls; update the sentence mentioning "llama3.2:latest"
so it clearly says it resumed processing (re-picked feature `#4`) rather than
asserting a successful end-to-end completion, and append a brief parenthetical
or clause noting that logs show plain-text tool calls and that other local
models failed to finish DreamForgeIdeas to make the verification record
consistent with the DreamForgeIdeas observations.
In `@scripts/agent-runner.mjs`:
- Around line 594-637: The fingerprintProjectDir function uses a Number
accumulator `sum` that can overflow; change the accumulator to a precision-safe
type (use BigInt) and update all per-entry additions to use BigInt (e.g.,
initialize sum = 0n, convert p.length and stat.size and Math.floor(stat.mtimeMs)
to BigInt before adding). Ensure the function returns the BigInt (or a stable
string representation) and update any comparisons that check `dirBefore.sum !==
dirAfter.sum` to compare the same type (or compare strings) so the confabulation
guard remains precision-safe; keep the existing `count` logic and SKIP_DIRS
behavior unchanged.
---
Duplicate comments:
In `@scripts/agent-runner.mjs`:
- Around line 701-717: The isPermanentError function currently treats any
errorMessage containing the substrings "401" or "403" as permanent, which is too
broad; update the permanentPatterns check in isPermanentError to require tighter
matches (e.g., match HTTP status tokens or words like "401", "401 Unauthorized",
"HTTP 401", and similarly for 403) by using word-boundary/regex checks or
explicit token matching instead of simple substring includes; modify the logic
in isPermanentError to test those patterns with a regex or by splitting the
message into tokens so only true HTTP status indications (e.g., /\b401\b/,
/\b403\b/, /\b401 Unauthorized\b/, /\b403 Forbidden\b/) trigger a permanent
error, leaving other patterns (like "invalid api key", "unknown model")
unchanged.
---
Nitpick comments:
In `@scripts/agent-runner.mjs`:
- Around line 1139-1143: Create a shared constant for the "recommended models"
guidance and replace the hardcoded message here and the other
confabulation-guard message with that constant to keep guidance consistent;
e.g., define a clear identifier like RECOMMENDED_TOOL_MODELS_GUIDANCE near the
top of scripts/agent-runner.mjs (or in an imported constants module) and use it
in the emitLog call (and the other occurrence) so both failure modes reference
the exact same string while preserving the existing log level and formatting.
🪄 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
Run ID: f1e1749d-19d8-423c-9145-3968ebf7ad63
📒 Files selected for processing (4)
README.mddev/bugtracker.mddocs/example-app-features.jsonscripts/agent-runner.mjs
✅ Files skipped from review due to trivial changes (1)
- docs/example-app-features.json
Comment on lines
+594
to
+637
| function fingerprintProjectDir(projectDir) { | ||
| const SKIP_DIRS = new Set([".pi", "node_modules", ".git", ".next"]); | ||
| let count = 0; | ||
| let sum = 0; | ||
| function walk(dir, depth) { | ||
| if (depth > 6 || count > 10000) return; | ||
| let entries; | ||
| try { | ||
| entries = fs.readdirSync(dir, { withFileTypes: true }); | ||
| } catch { | ||
| return; | ||
| } | ||
| for (const e of entries) { | ||
| if (count > 10000) return; | ||
| const name = e.name; | ||
| const p = path.join(dir, name); | ||
| if (e.isDirectory()) { | ||
| // Track presence of skipped directories themselves (their existence | ||
| // is a meaningful signal — `node_modules` appearing after a session | ||
| // means `npm install` ran) but don't descend into them. | ||
| if (SKIP_DIRS.has(name)) { | ||
| count++; | ||
| sum += p.length + 1; | ||
| continue; | ||
| } | ||
| walk(p, depth + 1); | ||
| } else { | ||
| try { | ||
| const stat = fs.statSync(p); | ||
| count++; | ||
| sum += p.length + stat.size + Math.floor(stat.mtimeMs); | ||
| } catch { | ||
| /* deleted between readdir and stat — ignore */ | ||
| } | ||
| } | ||
| } | ||
| } | ||
| try { | ||
| walk(projectDir, 0); | ||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| return { count, sum }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the JavaScript Number.MAX_SAFE_INTEGER and understand the accumulation
node -e "console.log('Number.MAX_SAFE_INTEGER:', Number.MAX_SAFE_INTEGER); console.log('Approx April 2026 timestamp ms:', new Date('2026-04-01').getTime()); console.log('10000 * 1.7e12:', 10000 * 1.7e12);"Repository: leonvanzyl/localforge
Length of output: 187
🏁 Script executed:
# Now let's examine the full context of fingerprintProjectDir and how it's used
cd "$(git rev-parse --root)" && cat -n scripts/agent-runner.mjs | sed -n '594,680p'Repository: leonvanzyl/localforge
Length of output: 147
🏁 Script executed:
# Search for where fingerprintProjectDir is called and how the returned sum is used
rg "fingerprintProjectDir|\.sum\s*[!=]" scripts/agent-runner.mjs -A 3 -B 3Repository: leonvanzyl/localforge
Length of output: 1001
🏁 Script executed:
# Check for the isPermanentError function and the 401/403 matcher mentioned in the scratchpad
rg "isPermanentError|401|403" scripts/agent-runner.mjs -B 2 -A 5Repository: leonvanzyl/localforge
Length of output: 902
Use a precision-safe accumulator for the workspace fingerprint.
At the current 10,000-file cap, accumulating p.length + stat.size + Math.floor(stat.mtimeMs) per file exceeds Number.MAX_SAFE_INTEGER (≈9e15) with realistic modern workspaces at ≈1.7e12 ms per timestamp. When sum overflows, the equality check dirBefore.sum !== dirAfter.sum loses precision, making the confabulation guard miss real file writes or report false unchanged states. Switch this fingerprint to BigInt or a hash string before comparing snapshots.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/agent-runner.mjs` around lines 594 - 637, The fingerprintProjectDir
function uses a Number accumulator `sum` that can overflow; change the
accumulator to a precision-safe type (use BigInt) and update all per-entry
additions to use BigInt (e.g., initialize sum = 0n, convert p.length and
stat.size and Math.floor(stat.mtimeMs) to BigInt before adding). Ensure the
function returns the BigInt (or a stable string representation) and update any
comparisons that check `dirBefore.sum !== dirAfter.sum` to compare the same type
(or compare strings) so the confabulation guard remains precision-safe; keep the
existing `count` logic and SKIP_DIRS behavior unchanged.
brainit-consulting
force-pushed
the
pr/permanent-error-and-perf
branch
from
April 29, 2026 21:29
609915f to
ba207f3
Compare
brainit-consulting
changed the title
…NH-007) Live verification with gpt-oss:20b on Ollama exposed a failure mode the existing guards don't catch: the agent completes the real work in the first few minutes, then enters a loop of hallucinated/no-op tool calls (repeated `Read package.json`, `Listing files`, made-up tool names like `feature_get_ready`) until the 30-minute session watchdog kills the runner. The watchdog's `terminated` outcome demotes the feature back to backlog despite the work being legitimately done — the next 25 minutes of compute are wasted, AND credit for the completed work is lost. This change adds an idle-stop heuristic that runs in parallel with the existing 30-minute watchdog: - Every IDLE_CHECK_INTERVAL_MS (30s) re-fingerprint the project working directory using the existing fingerprintProjectDir() helper. If the fingerprint moved, bump lastFsChangeAt = Date.now(). - Hook the existing session.subscribe() to track bash tool start/end timestamps. While bash is in flight, idle-stop is skipped so a long-running install (npm install, drizzle migrate) isn't cancelled before it can produce files. - If neither has moved in the last LOCALFORGE_IDLE_STOP_MS milliseconds (default 300_000 = 5 minutes), set idleStopTriggered = true, emit a user-facing log line, and call session.abort(). - After the prompt promise resolves, if idleStopTriggered is set, clear errorMessage and reset resultSubtype to "success" so the regular ok-computation runs as if the session ended normally. The result-override is critical: idle-stop is purely an early-termination optimization, not a guard bypass. The downstream confabulation guards (tool-call floor, fs-fingerprint, streak escalation) still gate the final outcome: - work was done before the idle stretch → fs changed → guards pass → feature completes legitimately - no work ever happened → fs unchanged → fs-fingerprint guard rejects with the existing confabulation message → feature demoted as before Configurable via LOCALFORGE_IDLE_STOP_MS; set to 0 to disable and fall back to the 30-minute watchdog only. Implementation passes typecheck + lint clean. Live verification on the same gpt-oss:20b scenario that surfaced the failure is the next step.
…007 live
Two related additions surfaced during live ENH-007 verification:
1. Ollama returns `500 model requires more system memory (X GiB) than
is available (Y GiB)` when the configured model can't fit in
currently-free RAM. This is the same shape as BUG-001's
"does not support tools" — retrying immediately won't help; the
user has to free RAM or pick a smaller model. Without recognising
it as permanent the orchestrator auto-continues and tight-loops
on the same memory error.
- isPermanentError() now matches `requires more system memory`,
`out of memory`, `cuda out of memory`.
- The user-facing guidance message branches: memory errors get
"close heavy apps, or run `ollama stop <other-model>`, or pick
a smaller model" wording. Tool-support errors keep the existing
switch-models guidance.
- New helper isMemoryExhaustionError() encapsulates the sub-match
so the branch is easy to read and extend.
2. ENH-007 (idle-stop heuristic) is now VERIFIED end-to-end live.
Tested 2026-04-29 with gpt-oss:20b on Ollama and
LOCALFORGE_IDLE_STOP_MS=60000 (1-minute threshold for a
deterministic test). Session 665 ran ~5 min total: bash created
23 real files (Next.js scaffold), agent went idle, idle-stop
fired at the 60-s mark, override cleared the `aborted` result,
the fingerprint guard saw `dirChanged: true` and passed, the
feature was marked completed. Auto-continue picked up #75
(db-setup) cleanly. Earlier sessions on the same run also
exercised the confabulation-rejection path — idle-stop fires,
override runs, fingerprint guard correctly demotes the
no-fs-change session. Idle-stop does NOT bypass the safety
check; it only saves wall time on both paths.
dev/bugtracker.md updated with the full session-665 trace as
verification evidence and an extended classifier note under BUG-001
covering the memory-exhaustion patterns.
brainit-consulting
changed the title
brainit-consulting
added a commit
to brainit-consulting/localforge
that referenced
this pull request
Apr 30, 2026
…am as leonvanzyl#4) # Conflicts: # README.md
- package.json: add engines.node >=20 to fail fast on older runtimes. - lib/agent/orchestrator.ts: __resetStateForTests now clears the permanentlyBlocked set and confabulationCounts map so tests can't leak permanent-failure / streak state across cases. - lib/agent/orchestrator.ts: rephrase the permanent-failure log so it matches the actual retry path — clicking Run queue clears the in-process blocklist; "paused until next restart" was inaccurate. - app/globals.css: replace undefined var(--ink-1) with var(--ink) in the .tb-model rules so the model badge renders correctly. - scripts/agent-runner.mjs: tighten the 401/403 patterns in isPermanentError to a word-boundary regex (\b(401|403)\b) so unrelated digit substrings (model ids, port numbers, payload bytes) can't misclassify transient errors as permanent. - components/forge/app-shell.tsx: collapse the duplicate useActiveProject() call into a single destructure so refreshTick and the rest of the context come from the same call site. - docs/example-app-features.json + docs/example-app-requirements.md: Next 15 → Next 16 (matches the repo baseline) and add `text` language tags to the unlabeled fenced code blocks (markdownlint MD040).
brainit-consulting
added a commit
to brainit-consulting/localforge
that referenced
this pull request
Apr 30, 2026
- package.json: add engines.node >=20 to fail fast on older runtimes. - lib/agent/orchestrator.ts: __resetStateForTests now clears the permanentlyBlocked set and confabulationCounts map so tests can't leak permanent-failure / streak state across cases. - lib/agent/orchestrator.ts: rephrase the permanent-failure log so it matches the actual retry path — clicking Run queue clears the in-process blocklist; "paused until next restart" was inaccurate. - app/globals.css: replace undefined var(--ink-1) with var(--ink) in the .tb-model rules so the model badge renders correctly. - scripts/agent-runner.mjs: tighten the 401/403 patterns in isPermanentError to a word-boundary regex (\b(401|403)\b) so unrelated digit substrings (model ids, port numbers, payload bytes) can't misclassify transient errors as permanent. - components/forge/app-shell.tsx: collapse the duplicate useActiveProject() call into a single destructure so refreshTick and the rest of the context come from the same call site. - docs/example-app-features.json + docs/example-app-requirements.md: Next 15 → Next 16 (matches the repo baseline) and add `text` language tags to the unlabeled fenced code blocks (markdownlint MD040).
…nt (BUG-006)
Pi's DefaultResourceLoader walks up from cwd looking for context
files. The default LocalForge install layout puts project folders
under the harness repo (H:\localforge\projects\<name>\), so the
loader hops two levels up and ingests the harness's own CLAUDE.md —
which is the user-facing project-assistant rulebook ("READ-only,
cannot run bash, cannot modify source code").
The coding agent then gets that rulebook merged into its system
context. Big well-aligned models (gpt-oss:20b in particular) take
the rulebook seriously and politely refuse to do anything; the agent
log surfaces lines like "I'm only a project-assistant and don't have
permission to execute shell commands". ENH-001's confabulation guard
catches the resulting toolCalls=0 rejection, but the user-visible
reason was wrong — it blamed the model when the real cause was the
leaked rulebook.
Smaller / less-aligned models (qwen2.5-coder:7b, llama3.2:*) ignored
the restrictions and confabulated tool calls anyway, which is why
the bug hid under the existing "small Ollama models confabulate"
pattern through every prior verification run.
Fix: pass `noContextFiles: true` to the runner's DefaultResourceLoader
constructor. The bootstrapper route already does this via the
createPiResourceLoader helper. The coding agent's actual instructions
come from buildCodingSystemPrompt() + the per-feature prompt file
the orchestrator writes; there's no use case where a parent-directory
CLAUDE.md is the right place for coding-agent rules.
Verified live 2026-04-30 on the Author Landing project: with the fix
absent, gpt-oss:20b refused the scaffold step ("I'm only a
project-assistant…"). Re-running the same step with the fix in place
will be the verification step.
…Author Landing run The previous verification summary in dev/bugtracker.md and the README "Model selection" section claimed gpt-oss:20b only worked for one-shot scaffold steps and that incremental edits were unreliable. That conclusion was contaminated by BUG-006 — Pi's resource loader was ingesting the harness's own CLAUDE.md (which tells the project- assistant Claude it cannot run bash or modify source code) into every coding-agent session. With BUG-006 now fixed in this PR (commit 3022789), we re-ran the test on a fresh 8-feature Author Landing backlog with gpt-oss:20b running CPU-spilled on an 8 GB GPU. The results: 4 features completed end-to-end by the agent (scaffold, db setup, schema migration, hero section with self-correcting build cycle), 4 by human handoff (from-scratch route handler, binary placeholder PNG, long verbatim text excerpt, fresh React component). Resulting `npm run build` is clean and the app functions. The actual capability ceiling is much higher than originally documented. Updated: - README "Model selection" — replaced the three-class taxonomy with a verified pattern table (CLI-driven and edit-existing succeed; from-scratch code, binary files, and long verbatim text are the documented hand-off cases). Added a footnote explicitly noting the prior gpt-oss:20b conclusions were contaminated by BUG-006. - dev/bugtracker.md verification summary — supersedes the 2026-04-29 summary with the 2026-04-30 Author Landing run as the primary evidence. Adds a "Critical correction first" preamble pointing at BUG-006 as the bug that masked the model's real ceiling. Reframes the contribution as the ENH-001 + ENH-007 + auto-continue retry stack working as a system, not as independent fixes.
brainit-consulting
added a commit
to brainit-consulting/localforge
that referenced
this pull request
Apr 30, 2026
…Author Landing run The previous verification summary in dev/bugtracker.md and the README "Model selection" section claimed gpt-oss:20b only worked for one-shot scaffold steps and that incremental edits were unreliable. That conclusion was contaminated by BUG-006 — Pi's resource loader was ingesting the harness's own CLAUDE.md (which tells the project- assistant Claude it cannot run bash or modify source code) into every coding-agent session. With BUG-006 now fixed in this PR (commit 3022789), we re-ran the test on a fresh 8-feature Author Landing backlog with gpt-oss:20b running CPU-spilled on an 8 GB GPU. The results: 4 features completed end-to-end by the agent (scaffold, db setup, schema migration, hero section with self-correcting build cycle), 4 by human handoff (from-scratch route handler, binary placeholder PNG, long verbatim text excerpt, fresh React component). Resulting `npm run build` is clean and the app functions. The actual capability ceiling is much higher than originally documented. Updated: - README "Model selection" — replaced the three-class taxonomy with a verified pattern table (CLI-driven and edit-existing succeed; from-scratch code, binary files, and long verbatim text are the documented hand-off cases). Added a footnote explicitly noting the prior gpt-oss:20b conclusions were contaminated by BUG-006. - dev/bugtracker.md verification summary — supersedes the 2026-04-29 summary with the 2026-04-30 Author Landing run as the primary evidence. Adds a "Critical correction first" preamble pointing at BUG-006 as the bug that masked the model's real ceiling. Reframes the contribution as the ENH-001 + ENH-007 + auto-continue retry stack working as a system, not as independent fixes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR hardens the orchestrator and agent runner against five classes of
local-model failure that LocalForge currently doesn't catch, plus a small
batch of UX and quality-gate fixes. All harness changes are verified live
across multiple Ollama models on two test backlogs (DreamForgeIdeas, 10
features, 2026-04-29; Author Landing, 8 features, 2026-04-30); evidence is
recorded in
dev/bugtracker.md.Headline: the harness no longer fakes completions, no longer
infinite-loops, no longer reports success on empty project directories,
no longer wastes the full 30-minute watchdog budget on agents that
silently stop progressing after doing real work, and no longer leaks
the harness's own project-assistant CLAUDE.md into the coding agent's
context — that last one is a critical bug surfaced during 2026-04-30
re-verification (BUG-006 below) which silently disabled the coding
agent for every default-layout install.
Critical correction up front
The 2026-04-29 verification round of this PR concluded that
gpt-oss:20bcould only handle one-shot scaffold steps and thatincremental edits were unreliable. That conclusion was contaminated
by BUG-006: Pi's resource loader was walking up from the project's
working directory and ingesting the harness's own
CLAUDE.md(whichtells the project-assistant Claude it cannot run bash or modify source
code) into every coding-agent session. Big well-aligned models like
gpt-oss:20btook the rulebook seriously and refused to do anything;smaller models ignored the restrictions and confabulated tool calls
anyway, which masked the underlying bug behind ENH-001's existing
"small Ollama models confabulate" pattern.
With BUG-006 fixed in this PR (commit
3022789), the actual capabilityceiling is much higher than the prior verification documented. The
README "Model selection" section and
dev/bugtracker.mdverificationsummary have been rewritten with ground-truth evidence from the
2026-04-30 Author Landing run (commit
f652333).What's fixed
gemma3:*on Ollama). Now classified as a permanent error so the feature is paused with guidance, blocklisted for the rest of the process, and a single explicit Run-queue click clears the blocklist after the user fixes the config. The classifier was extended after live verification surfaced Ollama's500 model requires more system memoryerror as the same shape: it now also matchesrequires more system memory,out of memory,cuda out of memory. The user-facing guidance message branches on the cause.React.memowith a content-shallow comparator.<select>had no accessible name. Addedaria-label.DefaultResourceLoaderwalks up from cwd looking for context files; the default install layout puts user projects at<harness>/projects/<name>/, so the loader hops two levels up and ingests<harness>/CLAUDE.md— the user-facing project-assistant rulebook ("READ-only, cannot run bash, cannot modify source code") — into the coding agent's system context.gpt-oss:20bthen politely refused to scaffold ("I'm only a project-assistant and don't have permission to execute shell commands"); smaller models ignored the rulebook and the failure was masked behind ENH-001's confabulation rejection. Fix: passnoContextFiles: trueto the runner's resource loader. Verified live 2026-04-30 — pre-fix gpt-oss:20b refused; post-fix the same model rannpx create-next-app .and produced a real Next.js project on disk. The bootstrapper route was already doing this; we missed it on the agent runner.LOCALFORGE_MIN_TOOL_CALLSandLOCALFORGE_REQUIRE_FS_CHANGES.LOCALFORGE_IDLE_STOP_MS=300000) that re-fingerprints the project working directory every 30s and tracks bash start/end timestamps. After 5 min of zero fs change AND zero bash activity (skipped while bash is in flight), it gracefully aborts the session, the result-override clears theabortedsubtype, and the existing fs-fingerprint guard makes the actual call — work was done before the idle stretch → completed; never any work → confabulation rejection.npm run lintwas silently failing since the Next 16 migration (Next 16 removednext lint). Addedeslint.config.mjs(flat config) and updated the npm script. The quality gate is back online.How these work as a system (Author Landing 2026-04-30 evidence)
The headline is no longer "each fix solves an isolated bug". The
Author Landing run made it clear that ENH-001 + ENH-007 +
auto-continue retry form a system that materially extends what a
flaky local model can finish. Repeated pattern observed across
multiple features:
claims done.
"agent claimed success but project working directory is unchanged"
error.
bash, completes legitimately.
This is the value: the harness gives a flaky model enough chances to
break through while refusing to fake completions. With BUG-006 fixed,
gpt-oss:20bon CPU-spilled 8 GB GPU completed 4 of 8 AuthorLanding features end-to-end (scaffold, db setup, schema migration,
hero with a self-correcting
npm run buildcycle). The remaining 4followed a predictable pattern (see README "Model selection") and
were handled via human handoff. The resulting
npm run buildisclean and the app functions.
Verification harness (also included)
docs/example-app-requirements.mdanddocs/example-app-features.json— DreamForgeIdeas spec + atomic backlog used 2026-04-29.
scripts/load-example-features.mjsandscripts/reset-project-features.mjs— POST/DELETE through the existing API; same code path as the UI.
Documentation
README.md"Model selection" section — replaced the three-classtaxonomy with a verified pattern table (CLI-driven and edit-existing
succeed; from-scratch code, binary files, and long verbatim text are
the documented hand-off cases). Includes a footnote explicitly
noting prior gpt-oss:20b conclusions were contaminated by BUG-006.
dev/bugtracker.md— verification evidence for every fix above plusa 2026-04-30 session summary at the top that supersedes the
2026-04-29 narrative.
Test plan
npx tsc --noEmitcleannpm run lintruns (reveals only pre-existing debt unrelated to this PR; none introduced here)gemma3:4b— single failure log, guidance, blocklist; switching to tool-capable model + Run queue clearsrequires more system memoryrecognised as permanentaria-label="Add a prerequisite feature"exposedgpt-oss:20bemitted "I'm only a project-assistant…" on the scaffold step; post-fix the same step rannpx create-next-app .and produced 13+ real files on diskllama3.2:latestsessionsgpt-oss:20b,qwen2.5-coder:7b,llama3.2:latestdirChanged: trueand passed, feature marked completed legitimately. Author Landing run repeatedly exercised the rejection path — idle-stop fires, fingerprint guard demotes the no-fs-change session, auto-continue retry frequently broke through on the next attemptnpm run lintproduces actionable outputgpt-oss:20bpost-BUG-006: 4/8 features completed by agent, 4 by human handoff (documented patterns: from-scratch code / binary files / long verbatim text), resultingnpm run buildclean, all routes resolveComing soon (planned follow-ups, not in this PR)
These are tracked in
dev/bugtracker.mdand will be filed as separate,narrowly-scoped PRs:
pr/triage-batch-2.fix/bootstrapper-confab-and-bypass-script.pr/triage-batch-2.gemma3or a 3B model.pr/triage-batch-2.pr/triage-batch-2.No breaking changes
All public APIs preserved. New env vars (
LOCALFORGE_MIN_TOOL_CALLS,LOCALFORGE_REQUIRE_FS_CHANGES,LOCALFORGE_IDLE_STOP_MS) default toenable-the-fix behaviour but can be set to
0for opt-out. Newoptional parameters on
findNextReadyFeatureForProjectandfinalizeSessiondefault to existing behaviour. The runner'sdoneevent gains optional
permanentandconfabulationfields — olderorchestrator code without the new handling would ignore them.