refactor(adapters): cut ~3,100 LOC of redundancy in the agent adapter layer (#2349)#2355
Open
harshitsinghbhandari wants to merge 4 commits into
Conversation
…r#2349) Introduce the shared building blocks the per-adapter cleanup will use: - ports.NormalizePermissionMode (finding 3) - hookutil.FileExists (finding 10) - binaryutil.ResolveBinary + BinarySpec (finding 2) - activitystate.StandardDeriveActivityState (finding 4) - agentbase.Base embed + StandardSessionInfo (findings 6-9) No adapters wired up yet; behavior unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…Wrapper#2349) Reference conversion proving the shared packages against real tests: - hooks.go collapses onto hooksjson.Manager (finding 1) - ResolveGooseBinary via binaryutil.BinarySpec (finding 2) - gooseMode uses ports.NormalizePermissionMode (finding 3) - activity.go deleted; dispatch points at activitystate (findings 4, 5) - SessionInfo via agentbase.StandardSessionInfo; Base embed drops the GetConfigSpec/GetPromptDeliveryStrategy no-ops (findings 6-8) - fileExists/atomicWriteFile copies removed (findings 5, 10) Also adds hooksjson package + activitystate test. goose: 327->~90 LOC. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pers (AgentWrapper#2349) Applies the shared helpers across every remaining adapter: - hooksjson.Manager for the matcher-group cohort (claudecode, qwen, droid) - binaryutil.BinarySpec for 19 binary resolvers (aider/cursor/opencode/codex keep their special resolvers; all now use hookutil.FileExists) - ports.NormalizePermissionMode replaces 15 private copies - agentbase.Base embed drops the GetConfigSpec/GetPromptDeliveryStrategy no-ops (and GetAgentHooks/GetRestoreCommand/SessionInfo no-ops on hookless adapters) - agentbase.StandardSessionInfo replaces the per-adapter metadata readers - activitystate.StandardDeriveActivityState via dispatch for the 8 name-only derivers; their activity.go files removed (claudecode/codex/droid/agy/opencode keep payload-parsing derivers) - 4 private atomicWriteFile copies missing fsync now use hookutil.AtomicWriteFile - 23 private fileExists copies removed Full backend build + vet + test suite green (1647 tests). Co-Authored-By: Claude Opus 4.8 <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.
Closes #2349.
Consolidates the copy-pasted logic across the 23 agent adapters in
backend/internal/adapters/agent/into a handful of shared helpers, with no behavioral change and one latent correctness fix (missingfsync).Net delta
-3,105 LOC of non-test production code (-3,454 total incl. tests): 78 files changed, 1,212 insertions, 4,666 deletions.
New shared packages
hooksjson— generic matcher-group hooks file (Read/Write/Install/Uninstall/AreInstalled). The claude/goose/qwen/droid hooks.go each collapse from ~180-380 LOC to ~60.binaryutil.ResolveBinary(ctx, BinarySpec)— table-driven binary search; per-adapter resolvers shrink to a ~10-lineBinarySpecliteral.agentbase.Base(embed) +agentbase.StandardSessionInfo— default no-opports.Agentmethods + the shared session-metadata reader.activitystate.StandardDeriveActivityState— the name-only hook→activity mapping shared viaactivitydispatch.ports.NormalizePermissionModeandhookutil.FileExists— the two most-copied one-liners.What was removed (findings from #2349)
hooksjson.Manager(claudecode, goose, qwen, droid)binaryutil.BinarySpec(19 resolvers)normalizePermissionMode×15ports.NormalizePermissionModeDeriveActivityState×8activitystate.StandardDeriveActivityState; activity.go deletedatomicWriteFile×7 (4 missing fsync)hookutil.AtomicWriteFileSessionInfoboilerplateagentbase.StandardSessionInfoGetPromptDeliveryStrategy/GetConfigSpec/ tier-C no-op stubsagentbase.BaseembedfileExists×23hookutil.FileExistsCorrectness fix: the private
atomicWriteFilecopies in autohand/cline/goose/kiro omittedtmp.Sync(); routing them throughhookutil.AtomicWriteFilerestores the fsync before rename.Kept deliberately separate (per #2349)
Verification
go build ./...,go vet ./..., andgo test ./...all green frombackend/— 1,647 tests pass across 84 packages. Per-adapter public behavior (launch/restore argv, hook file contents, session info, binary resolution) is unchanged; test expectations were only migrated where they referenced now-shared internal types.🤖 Generated with Claude Code