refactor: remove built-in formatter integration from write tools#1353
Merged
Conversation
github-actions
Bot
added
the
harness
Contributor
|
Warning Review limit reached
More reviews will be available in 17 minutes and 14 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (25)
✨ 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Astro-Han
added
the
task
Remove the auto-format-on-write behavior from edit, write, and apply_patch tools. This was the only agent tool among Codex CLI, Claude Code, and PawWork that auto-formatted files after writing — the other two leave formatting to the user (pre-commit hooks, CI, or optional PostToolUse hooks). Changes: - Remove format.file() calls from edit.ts, write.ts, apply_patch.ts - Delete format/ directory (index.ts, formatter.ts) — 20+ formatter defs - Remove Format wiring from AppLayer, BootstrapLayer, ToolRegistry - Remove /formatter API route from server - Remove formatter config schema field (with narrow compat shim) - Delete Bom.syncFile (dead code — only existed for post-format resync) - Update all related tests The AGENTS.md already states: 'Lint only, no formatter. Biome formatter is intentionally disabled.' This change aligns the code with that intent. Formatting remains the user's responsibility via pre-commit hooks, CI, or editor-level formatOnSave — consistent with how Codex CLI and Claude Code handle formatting.
Astro-Han
force-pushed
the
pawwork/remove-formatter-integration
branch
from
409c43d to
81ca9cb
Compare
Restore the dev-branch formatting in the touched files so the diff carries only formatter-removal changes (deleted module/imports/layers/route, config read-time compat shim, test rename/assertion, CI shard sync). The fork keeps Biome's formatter disabled, so dev's line wrapping is canonical; the prior revision had reflowed unrelated lines to a 120-column width. - config.ts: revert isWindowsSyncUnsupportedError, wellknown read, withLifecycleOrigin, dispose .catch, and writeConfigTextAtomic wrapping - registry.ts: revert isDeferredAvailable, availableDeferred, and the tool-info execute arrow wrapping - server/instance/index.ts: revert the two /vcs/diff description reflows - config.test.ts: revert readJson and provideTmpdirInstance reflows - apply_patch.ts: revert the change.after single-line reflow - bootstrap.ts: delete only format.init() from the boot list instead of collapsing the multi-line array Verification: bun run typecheck (opencode); bun test config.test.ts (110 pass) and apply_patch/edit/write tool tests (80 pass).
The /formatter route was removed from the server, but the SDK and a CI test still referenced it: SDK users could call a dead endpoint, Config.formatter kept misleading new configs, and unit-opencode failed. SDK (surgical removal, no compat shell — the checked-in openapi.json is a periodically-refreshed snapshot, not regenerated per change, so a full regen would churn ~20 unrelated routes): - openapi.json: remove the /formatter path, the FormatterStatus schema, and the Config.formatter property (deletions only) - v1 + v2 gen: remove the Formatter client class, the formatter getter on OpencodeClient, the FormatterStatus model, the FormatterStatus* request/ response types, the Config.formatter field, and the now-unused imports CI: - ci-workflow.test.ts: drop test/format from the windows-server-tools shard command so the pinned command matches windows-advisory.yml (which already dropped it), fixing the "defines Windows unit packages and opencode shards" assertion Verification: SDK typecheck; opencode typecheck; bun test ci-workflow.test.ts (17 pass) and route-inventory-harness.test.ts (11 pass); repo grep confirms no formatter API/config types remain outside docs and the negative-assertion config test.
With the formatter integration gone, nothing mutates a file between the write and the metadata build, so the recompute/relay scaffolding left behind is now pure dead code. - write.ts: drop the finalContent alias; it only ever equals contentNew - edit.ts: drop the second, byte-identical post-write diff recompute (the diff built before the write is already final) - apply_patch.ts: drop the write-only totalDiff variable, its three += appends, and the post-apply rebuild loop. The permission ask and the result already derive the aggregate via safeTotalDiff(fileChanges), and the per-file `files` metadata is fully populated at build time, so the rebuild re-assigned identical values. Verification: bun run typecheck (opencode); bun test test/tool/write.test.ts test/tool/edit.test.ts test/tool/apply_patch.test.ts (80 pass).
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
Remove the built-in auto-format-on-write behavior from
edit,write, andapply_patchtools. Delete the entireformat/subsystem (~600 lines): 20+ formatter definitions, service layer, config schema, API route, and tests. The removal also propagates to the public contract — the/formatterroute, the SDKFormatterclient andFormatterStatus*types, and theConfig.formatterfield are dropped fromopenapi.jsonand the v1/v2 SDK. This was the only agent tool among Codex CLI, Claude Code, and PawWork that auto-formatted files after writing — the other two leave formatting to the user's pre-commit hooks, CI, or optional PostToolUse hooks.Why
The built-in formatter created invisible side effects: agents writing
.tsfiles had their code reformatted behind the scenes, causing diff mismatches and agent confusion. The AGENTS.md already states "Lint only, no formatter. Biome formatter is intentionally disabled" — this change aligns the code with that intent.Related Issue
None.
Human Review Status
Pending
Review Focus
format.file()from the three write tools doesn't break the write-then-diff flownormalizeLoadedConfigsilences oldformatterconfig fields without erroringRisk Notes
formatteris removed from the public Config schema and the SDKConfigtype, andConfigis strict (additionalProperties: false). So$schema-aware editors and the SDK type now flag a staleformatterkey as invalid — this is the desired signal to drop the dead key. At runtime, the narrowdelete copy.formattershim innormalizeLoadedConfigstripsformatter: true/false/mapbefore the strict decode, so existing configs still load with the key silently ignored rather than hard-erroring on upgrade. Net: the contract no longer advertisesformatter(tooling rejects it), while old configs keep loading during migration.GET /formatter, theFormatterclient class andformattergetter, theFormatterStatus*request/response types, theFormatterStatusmodel, and theConfig.formatterfield are removed fromopenapi.jsonand both the v1 and v2 SDK. Any external consumer ofclient.formatter/GET /formattermust drop it. No internal consumers were found. The checked-inopenapi.jsonis a periodically-refreshed snapshot (not regenerated per change), so the formatter entries were removed surgically rather than via a full regen that would churn ~20 unrelated routes.How To Verify
Screenshots or Recordings
Not applicable — no visible UI changes.
Checklist
taskharness(auto-assigned by bot)P2(auto-assigned by bot)Pending.dev, and my PR title and commit messages use Conventional Commits in English.