Skip to content

feat(agents): add copilot adapter#128

Merged
harshitsinghbhandari merged 4 commits into
agents/08-qwenfrom
agents/09-copilot
Jun 6, 2026
Merged

feat(agents): add copilot adapter#128
harshitsinghbhandari merged 4 commits into
agents/08-qwenfrom
agents/09-copilot

Conversation

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 6, 2026

Greptile Summary

Adds the GitHub Copilot CLI agent adapter: a new copilot package with binary resolution, headless session launch/restore, workspace-local hook install/uninstall, and an activity deriver, all wired into the existing registry and dispatch table.

  • copilot/copilot.go: Plugin implementing adapters.Adapter + ports.Agent; resolves the copilot binary once per lifetime and maps AO permission modes onto Copilot CLI approval flags (--allow-tool write, --allow-all-tools, --allow-all).
  • copilot/hooks.go: Manages .github/hooks/ao.json — install, uninstall, idempotency, and atomic write — wired to four Copilot lifecycle events (sessionStart, userPromptSubmitted, permissionRequest, agentStop).
  • copilot/activity.go: DeriveActivityState maps the four hook sub-commands onto ActivityActive, ActivityIdle, and ActivityWaitingInput.

Confidence Score: 4/5

Safe to merge with one fix: the local atomicWriteFile in hooks.go is missing a Sync call, which weakens its crash-safety guarantee relative to the shared hookutil version.

The adapter is well-structured and thoroughly tested. The one issue is that the local atomicWriteFile in copilot/hooks.go omits the tmp.Sync() step that hookutil.AtomicWriteFile includes, meaning a crash after Close() but before the OS flushes buffers could corrupt the hooks file and silently disable all Copilot hooks. The fix is a one-liner and the rest of the adapter is clean.

backend/internal/adapters/agent/copilot/hooks.go — the atomicWriteFile implementation needs a Sync call to match the shared hookutil version.

Important Files Changed

Filename Overview
backend/internal/adapters/agent/copilot/hooks.go New file. Implements Copilot hook install/uninstall/check. Has its own atomicWriteFile that is missing the tmp.Sync() call present in the shared hookutil.AtomicWriteFile, reducing crash-safety guarantees compared to other adapters.
backend/internal/adapters/agent/copilot/copilot.go New file. Core adapter: binary resolution with once-per-lifetime caching, launch/restore command building, and session info. Logic is clean; noted limitations (nvm path, fallback caching) were flagged in previous review rounds.
backend/internal/adapters/agent/copilot/activity.go New file. Simple, well-documented event-to-activity-state mapper. No issues found.
backend/internal/adapters/agent/copilot/copilot_test.go New file. Good test coverage: launch/restore command building, permission mode mapping, hook install/uninstall idempotency, user-hook preservation, and activity deriver exhaustive table test.
backend/internal/adapters/agent/activitydispatch/dispatch.go Trivial one-line addition registering copilot.DeriveActivityState in the dispatch map. Correct.
backend/internal/adapters/agent/registry/registry.go Trivial one-line addition adding copilot.New() to Constructors(). Correct.
backend/internal/daemon/wiring_test.go Adds HarnessCopilot"copilot" resolver test case. Correct integration smoke test.

Sequence Diagram

sequenceDiagram
    participant AO as AO Daemon
    participant Plugin as copilot.Plugin
    participant FS as Filesystem
    participant Copilot as Copilot CLI

    AO->>Plugin: GetAgentHooks(cfg)
    Plugin->>FS: readCopilotHooks(.github/hooks/ao.json)
    FS-->>Plugin: copilotHookFile (or empty)
    Plugin->>Plugin: merge copilotManagedHooks (idempotent)
    Plugin->>FS: atomicWriteFile (tmp → rename)
    FS-->>Plugin: ok

    AO->>Plugin: GetLaunchCommand(cfg)
    Plugin->>Plugin: copilotBinary() [resolve once, cache]
    Plugin-->>AO: ["copilot", "--allow-all-tools", "-p", prompt]

    AO->>Copilot: exec(launch command)
    Copilot-->>FS: fires sessionStart hook
    FS->>AO: ao hooks copilot session-start → ActivityActive

    Copilot-->>FS: fires permissionRequest hook
    FS->>AO: ao hooks copilot permission-request → ActivityWaitingInput

    Copilot-->>FS: fires agentStop hook
    FS->>AO: ao hooks copilot stop → ActivityIdle

    AO->>Plugin: GetRestoreCommand(cfg)
    Plugin-->>AO: ["copilot", "--allow-all-tools", "--resume", agentSessionID]
    AO->>Copilot: exec(restore command)
Loading

Reviews (2): Last reviewed commit: "feat(agents): add copilot adapter" | Re-trigger Greptile

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>
@yyovil yyovil force-pushed the agents/09-copilot branch from 2be4aab to 2518bc2 Compare June 6, 2026 03:31
Comment thread backend/internal/adapters/agent/copilot/hooks.go
@harshitsinghbhandari harshitsinghbhandari added this to the rewrite milestone Jun 6, 2026
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yyovil has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

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>
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yyovil has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@harshitsinghbhandari
Copy link
Copy Markdown
Collaborator

Verified upstream Copilot CLI docs and pushed one fix.

Findings re-verified against upstream docs

Changes (f1f540d)

  • backend/internal/adapters/agent/copilot/hooks.go:88{Event: "permissionRequest", …}{Event: "preToolUse", …}; comment block above copilotManagedHooks now cites the upstream docs and documents the many-to-one collapse (AO sub-command stays permission-request; only the Copilot-facing JSON key changed).
  • backend/internal/adapters/agent/copilot/activity.go:13-19 — docstring updated so the permissionRequest/preToolUse distinction matches reality.
  • backend/internal/adapters/agent/copilot/copilot_test.go — added TestCopilotManagedHooksUseDocumentedEventNames as a tripwire pinning every JSON event key (sessionStart, userPromptSubmitted, preToolUse, agentStop) to its AO sub-command. Existing install/uninstall tests already iterate copilotManagedHooks, so they pick up the new key automatically.

Validation

  • cd backend && go build ./... — clean
  • cd backend && go test -race ./... — 797 passed in 50 packages (32 in copilot)

Pushed with --force-with-lease. No design changes; AO's internal ao hooks copilot <sub> vocabulary and the activity-state mapping are unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yyovil has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@harshitsinghbhandari harshitsinghbhandari merged commit fac804b into agents/08-qwen Jun 6, 2026
8 checks passed
harshitsinghbhandari added a commit that referenced this pull request Jun 6, 2026
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants