Harness upgrade: close the open loop (verification gate, self-correction) + 15 robustness/agentic items - #26
Merged
Merged
Conversation
Adds src/llm/history.ts (sanitizeHistory) and applies it in handleLlmChat before adapter dispatch. Strips UI-control sentinels (__DIFF__, __APPLIED__, __REDO__, __PLAN_CONFIRM__, __RETRY__) and folds load-bearing role:system feedback (rejections, failed matches, read/verify errors) into the current user turn so it survives every adapter's system-drop and reaches CLI agents. Closes the feedback half of the open loop. 10 new tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
extractWithStatus now reports which recovery layer succeeded; parseLlmResponse
returns {status, json, modifications, explanation} and is threaded through
handleLlmChat → llm.done payload. Toolbar surfaces a Retry affordance for
truncated/salvaged/failed parses instead of presenting a cut stream as success,
and warns when edits parsed from a possibly-incomplete response. 8 new tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New src/llm/thinking.ts resolvers (resolveMaxOutput, resolveReasoningLevel, resolveThinkingBudget, mapOpenAiEffort, mapGoogleThinkingLevel, coerceThinkingLevel). All three API adapters now: - lift max_tokens/maxOutputTokens from hardcoded 4096/8192 to the model's real registry maxOutput (multi-file edits no longer truncate mid-JSON), - read per-request context.reasoningLevel/thinkingBudget (overriding defaults), - detect finish_reason/stop_reason "length"/"max_tokens"/"MAX_TOKENS" and flag the response truncated → surfaces via H5's parseStatus. Server wires the previously-dead config.preferredThinkingMode into requests. Extracted buildAnthropicRequest/buildGoogleRequest as pure, tested functions. 17 new tests; reconciled 2 provider-execution tests to the raised caps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New src/abort-registry.ts tracks in-flight LLM requests keyed by connId:msgId. handleLlmChat and every adapter (3 API + 3 CLI) now accept an AbortSignal: API adapters pass it to fetch(); CLI adapters SIGTERM the child on abort. Server registers a controller per llm.chat, aborts on llm.cancel, and abortByPrefix on WebSocket close so a closed tab no longer leaves a fetch()/CLI child running (and editing files). ws-client sends llm.cancel on idle timeout and exposes cancelAllStreams(). 4 new tests; browser + node smoke tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- patch.ts: raise the fuzzy/trigram tier floor from 0.8 to FUZZY_MIN_CONFIDENCE (0.92) and refuse fuzzy matching entirely for multi-hunk groups (>1 replace), where a single wrong guess can silently corrupt one file. - security.ts: validateToken now uses crypto.timingSafeEqual with a length guard instead of plain === (avoids token-timing leak). - toolbar: surfaces sub-1.0 fuzzy-match confidence as a badge on the applied patch message so non-exact applies are visible. Reconciled the prior 0.8776 fuzzy test deliberately: a genuine high-confidence near-match (long block, 1-char typo) still applies; a borderline one is now refused; multi-hunk fuzzy is refused. 6 new/changed tests; 145 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PatchManifests are now persisted to ~/.openmagic/manifests/<groupId>.json (env-overridable for tests) so undo survives a restart/crash — rollbackPatchGroup reads from disk when the in-memory map is gone. Adds parentGroupId linking and rollbackChain(), which walks the chain from the latest self-correction round back to the original pre-edit content (the substrate H6 needs). rollbackApplied now returns its errors instead of swallowing them, and surfaces them in the apply result. Stale manifests (>7d) pruned on startup; fs.patch.rollback gains a chain flag. 3 new tests; 148 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…he loop) New src/verify.ts: after a patch group is applied it (1) re-parses touched JSON, (2) detects and runs the project's own typecheck/lint npm script (cancellable, 20s cap), and (3) attributes failures to the edit (output must mention a touched file) so pre-existing errors don't trigger a false revert. New fs.patch.verify op, cancellable via the H4 abort registry. Toolbar verifyAndSettle() runs after every apply/apply-all: on a real failure it reverts the chain (H8) and re-engages the model with the errors as a bounded (N=2) self-correction, otherwise it reloads. sendPrompt gains an isSelfCorrect flag so the budget resets only on fresh prompts. An edit that breaks the build is no longer reported as "Applied". 15 new tests (163 total); browser E2E passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
H2 — buildContext was forwarding only 7 of the ~20 element fields that prompts.ts:buildContextParts reads, so rules 12-16 (themeState, cssVariables, resolvedClasses, visibilityState, activeBreakpoints, reactProps, childrenLayout, matchedCssRules, …) always saw undefined. Now forwards the whole captured element under per-field byte caps (outerHTML 8k, textContent 2k). Snapshot test asserts every prompt-read field is present so this can't silently regress. H7 — installRuntimeErrorCapture(): window.onerror + unhandledrejection, plus scrapeErrorOverlay() for Next.js/Vite/webpack overlays — the only signal that catches "compiled but white-screened". Threaded through buildContext → LlmContext.runtimeErrors → a dedicated prompt section + rule 17. 3 new tests (166 total). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- project-grounding.ts no longer silently slices files to 12KB: truncated files
get an explicit marker ("[TRUNCATED N chars — request the rest with
NEED_FILE: <path>]") and a truncated flag, with marker room reserved so the
file still fits the context budget. Silent slices caused guaranteed
search-match failures on the cut region.
- sendPrompt's auto-retry loop gives NEED_FILE and SEARCH_FILES separate budgets
(5 and 4) under a total-iteration safety cap, instead of sharing one counter
where reading files starved codebase searches. 1 new test (167 total).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a search block fails to match (applyDiff or apply-all preview), the toolbar no longer just prints an error. repromptOnMatchFailure() re-reads the actual file(s), hands the model the real content plus the failure reason, and asks for a corrected exact search block — bounded by its own matchRetryRounds counter (reset on each fresh prompt). Builds on H1 so the feedback reaches the model. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Plan-into-edit: runPlanBeforeEdit stores the plan (state.lastPlan); confirm-plan now prepends "Approved implementation plan: …" to the edit-pass user message so the plan reaches all providers (including CLI agents, which only read the last user turn) instead of being silently re-derived. - detectRepoConventions(): runtime-built summary (framework/router, package manager, TS strictness, component location, styling) added to the ground result and surfaced as a "Repo Conventions" prompt section. - Added a clarify-on-ambiguity rule (1b) to the system prompt. 1 new test (168). Browser E2E passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- killPortProcess now resolves listener PIDs via findListenerPids(), which uses netstat on Windows and lsof on POSIX — port cleanup previously was a silent no-op on win32. - Fixed grounded-files state divergence: a file pulled in via NEED_FILE retry now updates state.groundedFiles/groundedFileReasons so the context chips and later prompts match what was actually sent. Note: the subsystem map's "CSP stripped unconditionally / Content-Length not recomputed" proxy findings were verified as non-issues — CSP is stripped only inside the isHtml-injection branch and Content-Length is dropped with it (chunked), so the proxy was left unchanged. Anthropic non-dataURL image handling was already addressed in H13. Codex file-change capture is folded into H9. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New src/llm/tools.ts (tool specs + executeServerTool backed by the sandboxed read/grep/list handlers + per-provider serialize/extract) and src/llm/tool-loop.ts (provider-neutral runToolLoop + OpenAI/Anthropic/Google drivers). Tool-capable models read EXACT untruncated file contents via read_file/search_code/list_dir, then finalize via propose_edits — collapsing the dominant search-mismatch failure of the hand-written JSON contract. Non-streaming tool calls (simpler/safer than assembling tool_use deltas). Gated opt-in (config.useTools or OPENMAGIC_TOOLS=1) and bounded (12 steps), cancellable via the H4 signal; on any failure it falls back to the existing JSON-string path, which stays for non-tool models. H12 (full server-side orchestration rewrite) deferred per decision. 16 new tests (184 total); browser E2E passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New src/llm/native-capture.ts: when enabled (config.nativeEdit / OPENMAGIC_NATIVE_EDIT=1) and the project is a clean git repo, CLI agents (Claude Code/Codex/Gemini) get a plain task prompt (NATIVE_EDIT_INSTRUCTION, not the JSON contract) and edit the working tree natively + self-verify. The proxy then captures their diff via git, REVERTS the tree, and routes the changes through the normal review→apply→verify pipeline as ordinary edit/create/delete modifications. This reclaims the CLI agents' multi-turn editing power while AVOIDING the write-before-approve security regression (files are reverted before you review). Off by default; non-git/dirty trees fall back to the JSON-diff contract. Removes the double-apply. Cancellable via the H4 signal. 5 new tests (189 total). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- runVerifyScript: `npm run <script>` spawns the command as a grandchild, so SIGTERM to the npm wrapper left it alive on Linux and 'close' never fired (the test timed out at 5s on CI, passed on macOS). Now spawns detached and kills the whole process group (taskkill /T on Windows), resolving immediately on timeout/abort. Real fix: a timed-out verify no longer leaks a typecheck process. - npm audit fix: bump ws 8.20.0 → 8.21.0 (GHSA-58qx-3vcg-4xpx, moderate), within the existing ^8.18.0 range — lockfile only. 0 vulnerabilities. 189 tests, build, both smoke tests, and npm audit --omit=dev all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Closes OpenMagic's single biggest gap — the open loop — and lands the full pragmatic harness-upgrade path (ROADMAP.md + HARNESS.md, reconciled against the live source). Before this PR an edit was declared "Applied" the instant it was written, with zero verification; feedback that could close the loop was dropped by every adapter; and 71% of the captured DOM perception never reached the model. All of that is fixed here.
Built test-first throughout (red → green → refactor), verified after every item with
typecheck+ the full suite, and validated end-to-end against the real running web app (browser E2E + node smoke) at each tier boundary.npm run check:allgreen: typecheck · lint · 189 tests (was 99, +90) · node smoke · browser E2E ·npm packWhat changed
Loop-closing (the headline)
src/verify.ts+fs.patch.verify: after apply, re-parse touched JSON, then run the project's own typecheck/lint (capability-probed, 20s cap, cancellable). Failures attributed to the edit (output must mention a touched file → no false reverts on pre-existing errors) trigger a chain rollback + a bounded (N=2) self-correction with the errors fed back. An edit that breaks the build is no longer reported as success.~/.openmagic/manifests/(survive restart),parentGroupIdlinking +rollbackChain()(restores last-working state),rollbackAppliedreturns its errors, stale manifests pruned on startup.sanitizeHistory()strips UI sentinels and folds rejections / failed-match / verify errors into the current user turn, so they survive every adapter'srole:systemdrop and reach CLI agents.window.onerror+unhandledrejection+ framework error-overlay scrape (Next.js / Vite / webpack) → prompt context. The only signal that catches "compiled but white-screened."Robustness
AbortSignalthreaded through every adapter →fetch()/CLI child;llm.cancel+ abort-on-disconnect (no more orphaned requests burning tokens / editing files).clean | salvaged | truncated | failed) with a Retry affordance instead of prose-as-success.maxOutput; per-request reasoning level;finish_reason: length→ truncated.crypto.timingSafeEqualfor the session token.Grounding & UX
Agentic (opt-in, off by default)
config.useTools/OPENMAGIC_TOOLS=1): tool-capable models read exact file contents viaread_file/search_code/list_dirand finalize viapropose_edits; bounded + cancellable; JSON-string path kept as fallback.config.nativeEdit/OPENMAGIC_NATIVE_EDIT=1): CLI agents edit the tree natively, then the diff is captured via git and reverted, routed through the normal review→apply→verify path — their multi-turn power without the write-before-approve regression. Requires a clean git tree; non-git/dirty falls back to the JSON contract.Hardening
netstatfallback), grounded-state sync after retry reads.Deferred (by decision)
agent.runorchestration rewrite) — a separate follow-up milestone; the loop is already closed via H6.Notes
Test plan
npm run check:all(typecheck, lint, 189 unit/integration tests, node smoke, browser E2E via Playwright,npm pack) — all green.🤖 Generated with Claude Code