Commit Graph

3 Commits

Author SHA1 Message Date
Harshit Singh Bhandari 2c08597ee6
refactor(adapters): cut ~3,100 LOC of redundancy in the agent adapter layer (#2349) (#2355)
* refactor(adapters): add shared helpers for adapter dedup (#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>

* refactor(goose): convert to shared adapter helpers (reference) (#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>

* refactor(adapters): dedup remaining 22 agent adapters onto shared helpers (#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>

* chore: format with prettier [skip ci]

* fix(binaryutil): preserve per-adapter Windows candidate order (#2349)

Review feedback: the shared resolver hardcoded Windows candidate order as
APPDATA then LOCALAPPDATA, which flipped Kiro's lookup so the npm shim
(%APPDATA%\npm\kiro-cli.*) was probed before the native install
(%LOCALAPPDATA%\Programs\kiro\kiro-cli.exe). A fixed order can't preserve every
adapter's original order (goose/vibe want APPDATA first, kiro wants LOCALAPPDATA
first), so BinarySpec now takes an ordered WinPaths []WinPath list where each
entry names its base (WinAppData/WinLocalAppData/WinHome). Every adapter's list
reproduces its pre-refactor order exactly; kiro's native path is restored to
first.

go build / vet / test all green (1647 tests).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(adapters): drop unused resolvedBinary writes flagged by govet (#2349)

Embedding agentbase.Base (value receiver) lets govet's unusedwrite analyzer
prove that setting resolvedBinary in tests that only call Base-promoted methods
(GetConfigSpec/GetPromptDeliveryStrategy/SessionInfo/cancellation) is a dead
write. Drop the field from those 23 constructions; GetLaunchCommand/GetRestore
tests that actually read it keep it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* style: gofmt manager_test.go struct alignment (#2349)

Inherited via the merge of main: a SessionRecord literal aligned its Metadata
field across a multi-key line, which gofmt/goimports rejects. One-line reformat
to unblock CI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(adapters): fix stale resolver fallback comments (#2349)

Review nit: 6 Resolve*Binary functions now delegate to binaryutil.ResolveBinary,
which returns a wrapped ports.ErrAgentBinaryNotFound rather than the bare binary
name. Update their doc comments (kiro, vibe, amp, agy, crush, cline) to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
2026-07-04 20:49:15 +05:30
Ashish Huddar 40bd2dfb2b
fix(sessions): stop AO hook files from making every worktree permanently dirty (#169)
* fix(sessions): stop AO hook files from making every worktree permanently dirty

Agent adapters write hook files (.codex/hooks.json, .opencode/plugins/
ao-activity.ts, .claude/settings.local.json, ...) into fresh session
worktrees as untracked files. `git worktree remove` (deliberately run
without --force) refuses on any untracked file, so Workspace.Destroy
failed for every session of the 12 workspace-writing harnesses:
POST /sessions/{id}/kill returned an unlogged 500 INTERNAL_ERROR and
`ao session cleanup` reported 'Would clean N' then '0 sessions cleaned'
with no reason, leaking workspaces forever.

Three coordinated fixes, none of which force-deletes user/agent work:

- Root cause: every adapter now writes a sentinel-guarded, self-ignoring
  .gitignore next to its hook files (hookutil.EnsureWorkspaceGitignore),
  so AO's own files no longer count as dirt while anything an agent
  drops — even in the same directory — still blocks teardown. A
  registry-wide conformance test enforces the contract for all current
  and future adapters. (Per-worktree .git/worktrees/<name>/info/exclude
  was evaluated first but git does not honor it.)
- Typed refusal: gitworktree.Destroy classifies a still-dirty refusal as
  ports.ErrWorkspaceDirty (git status probe). Kill maps it to success
  with freed=false (session terminated, worktree preserved); Cleanup
  reports it per-session as skipped-with-reason through the API
  (CleanupSessionsResponse.skipped), and the CLI prints
  'Skipped: <id> (workspace has uncommitted changes)' plus a summary.
- Observability: envelope.WriteError records the raw service error into
  a request-scoped slot and the access log attaches it to 5xx lines, so
  any remaining internal error is diagnosable server-side.

Worktrees created before this fix gain the .gitignore on restore (hook
install re-runs); their cleanup is otherwise reported as skipped instead
of erroring.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(cleanup): address Greptile P2s — surface dirty-probe failures, stop leaking raw errors

Two review findings on this PR:

- gitworktree.Destroy: when the isDirty probe itself failed, the error was
  silently discarded and the refusal looked identical to "registered but not
  dirty". The probe failure now rides the returned error (dirty probe: ...),
  so it reaches the access log via the 5xx error capture.

- Cleanup skip reasons: a non-dirty teardown failure put the raw error —
  including internal filesystem paths — into the public skipped[].reason
  field. The public reason is now the fixed string "workspace teardown
  failed"; the full cause goes to the daemon log (warn, with sessionID and
  path). The dirty-refusal reason is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(gitworktree): wrap the dirty-probe error with %w per errorlint

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-06-11 11:06:45 +05:30
Harshit Singh Bhandari b13f413515
feat(agents): add grok, cursor, qwen, copilot, kimi adapters (#145)
* feat(agents): add grok adapter

Registers the grok harness (xAI Grok CLI). grok installs Claude Code-compatible
hooks, so it reuses the claude-code activity deriver already in the platform.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(agents): add cursor adapter

Registers the cursor harness, stacked on the agent platform. Includes its own activity deriver.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(agents): add qwen adapter

Registers the qwen harness, stacked on the agent platform. Includes its own activity deriver.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(agents): add copilot adapter (#128)

* feat(agents): add copilot adapter

Registers the copilot harness, stacked on the agent platform. Includes its own activity deriver.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Update backend/internal/adapters/agent/copilot/hooks.go

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* fix(copilot): map permission-request to documented preToolUse event

Copilot CLI does not document a "permissionRequest" hook event. Per
https://docs.github.com/en/copilot/how-tos/copilot-cli/customize-copilot/use-hooks
the documented camelCase events are sessionStart, sessionEnd,
userPromptSubmitted, preToolUse, postToolUse, errorOccurred, agentStop.
Writing "permissionRequest" into .github/hooks/ao.json silently disables
that hook because Copilot does not recognize the key.

Remap AO's permission-request sub-command onto preToolUse (the closest
documented signal — fires before any tool invocation, including ones
that would prompt for approval) and add a tripwire test asserting the
JSON keys AO writes match the documented camelCase names.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* chore(copilot): gofmt the new tripwire test

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Harshit Singh Bhandari <dev@theharshitsingh.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: harshitsinghbhandari <24b4506@iitb.ac.in>

* feat(agents): add kimi adapter

Registers the kimi harness, stacked on the agent platform.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agents/kimi): drop approval flags on -p and --session paths

Kimi rejects `--prompt` combined with `--yolo`/`--auto`/`--plan`, and
rejects `--yolo`/`--auto` combined with `--session`/`--continue`
(non-interactive and resumed sessions inherit the auto permission
policy). The previous mapping appended one of those flags before `-p`
on every launch and before `--session` on every restore, so every
non-interactive launch would fail at startup. The local binary
(v1.37.0) additionally has no `--auto` option at all, which would
fail even on otherwise-permissible paths.

- GetLaunchCommand: emit approval flags only on the interactive path
  (no prompt). The `-p <prompt>` path is now bare.
- GetRestoreCommand: never emit approval flags; resumed sessions
  inherit the original session's approval settings.
- Tests assert no approval/plan flag leaks onto either path for any
  PermissionMode, and keep the interactive mapping unchanged.

Refs: https://moonshotai.github.io/kimi-code/en/reference/kimi-command.html

* fix(agents/qwen): sync hook settings temp file

* fix(agents/grok): delegate hook cleanup lifecycle

---------

Co-authored-by: yyovil <itsyyovil@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
2026-06-07 01:17:53 +05:30