Conversation
Entire-Checkpoint: c74816392558
PR SummaryMedium Risk Overview Extends transcript analysis and summarization to understand Copilot’s Written by Cursor Bugbot for commit a189b2e. Configure here. |
There was a problem hiding this comment.
Pull request overview
Adds GitHub Copilot CLI as a new (preview) agent integration in Entire CLI, aligning Copilot’s hook/transcript formats with the existing agent abstraction so sessions can be checkpointed and summarized consistently.
Changes:
- Introduces a new
agent/copilotimplementation (hook installation, lifecycle parsing, transcript parsing, tests). - Extends summarization to support Copilot
events.jsonltranscripts. - Updates setup/uninstall and hook command registration to include Copilot alongside existing agents.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| copilot-integration.md | Implementation plan/spec for Copilot agent integration and hook mapping. |
| cmd/entire/cli/summarize/summarize.go | Adds Copilot transcript parsing path for condensed transcript generation. |
| cmd/entire/cli/strategy/rewind_test.go | Registers Copilot agent for rewind strategy tests via blank import. |
| cmd/entire/cli/setup_test.go | Registers Copilot agent for setup tests via blank import. |
| cmd/entire/cli/setup.go | Updates uninstall flow to enumerate/remove hooks for any installed agent (incl. Copilot). |
| cmd/entire/cli/hooks_cmd.go | Ensures Copilot agent is registered so hook subcommands can be generated. |
| cmd/entire/cli/hook_registry.go | Categorizes Copilot hook verbs into tool/subagent types for hook logging. |
| cmd/entire/cli/agent/registry.go | Adds AgentNameCopilot / AgentTypeCopilot constants. |
| cmd/entire/cli/agent/copilot/types.go | Defines Copilot hook-config types, hook stdin structs, and events.jsonl transcript structs/constants. |
| cmd/entire/cli/agent/copilot/copilot.go | Implements the Copilot agent (identity, detection, legacy hook/session APIs). |
| cmd/entire/cli/agent/copilot/lifecycle.go | Implements Copilot hook → normalized lifecycle event translation + transcript analyzer methods. |
| cmd/entire/cli/agent/copilot/hooks.go | Installs/uninstalls Copilot hooks in .github/hooks/copilot-setup.json. |
| cmd/entire/cli/agent/copilot/transcript.go | Parses Copilot events.jsonl and extracts prompts/summary/modified files. |
| cmd/entire/cli/agent/copilot/*_test.go | Adds unit tests for Copilot agent behavior, hooks, lifecycle parsing, and transcript parsing. |
Comments suppressed due to low confidence (1)
cmd/entire/cli/agent/copilot/hooks.go:60
- The InstallHooks() docstring says existing Entire hooks are removed only when force=true, but the implementation removes existing Entire hook entries unconditionally (after the early idempotency return). Either update the comment to match the behavior, or gate the removal logic behind force if that was the intent.
// InstallHooks installs Copilot hooks in .github/hooks/copilot-setup.json.
// If force is true, removes existing Entire hooks before installing.
// Returns the number of hooks installed.
| raw, err := agent.ReadAndParseHookInput[userPromptRaw](stdin) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return &agent.Event{ | ||
| Type: agent.TurnStart, | ||
| Prompt: raw.Prompt, | ||
| Timestamp: time.Now(), | ||
| }, nil |
There was a problem hiding this comment.
TurnStart events require a non-empty SessionID in the lifecycle dispatcher, but Copilot's user-prompt-submitted hook parsing currently emits agent.TurnStart with an empty SessionID/SessionRef. This will cause DispatchLifecycleEvent to return an error ("no session_id in TurnStart"), potentially breaking Copilot hook execution. Consider treating this hook as pass-through (return nil) until sessionId/transcriptPath are available, or populate SessionID via another reliable mechanism before emitting TurnStart.
| raw, err := agent.ReadAndParseHookInput[userPromptRaw](stdin) | |
| if err != nil { | |
| return nil, err | |
| } | |
| return &agent.Event{ | |
| Type: agent.TurnStart, | |
| Prompt: raw.Prompt, | |
| Timestamp: time.Now(), | |
| }, nil | |
| // We intentionally treat this hook as a pass-through because it does not | |
| // provide sessionId or transcriptPath. Emitting a TurnStart event without | |
| // a SessionID would cause DispatchLifecycleEvent to fail. | |
| _, err := agent.ReadAndParseHookInput[userPromptRaw](stdin) | |
| if err != nil { | |
| return nil, err | |
| } | |
| return nil, nil |
| configKey := hookNameToConfigKey[HookNameSessionStart] | ||
| expectedCmd := cmdPrefix + HookNameSessionStart | ||
| if entries, ok := config.Hooks[configKey]; ok { | ||
| for _, entry := range entries { | ||
| if entry.Bash == expectedCmd { | ||
| return 0, nil // Already installed with same mode | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
InstallHooks()'s idempotency check only verifies that the expected command exists under the sessionStart config key. If some of the other 7 hooks are missing/misconfigured, this will incorrectly return 0 (no-op) and leave the installation in a partially broken state. Consider validating that all required hook keys contain an entry matching the expected command for the current mode before returning 0.
| configKey := hookNameToConfigKey[HookNameSessionStart] | |
| expectedCmd := cmdPrefix + HookNameSessionStart | |
| if entries, ok := config.Hooks[configKey]; ok { | |
| for _, entry := range entries { | |
| if entry.Bash == expectedCmd { | |
| return 0, nil // Already installed with same mode | |
| } | |
| } | |
| } | |
| allHooksPresent := true | |
| for _, hookName := range c.HookNames() { | |
| configKey, ok := hookNameToConfigKey[hookName] | |
| if !ok { | |
| // Skip hook names that don't have a corresponding config key, | |
| // matching the behavior used during installation below. | |
| continue | |
| } | |
| expectedCmd := cmdPrefix + hookName | |
| entries, exists := config.Hooks[configKey] | |
| if !exists { | |
| allHooksPresent = false | |
| break | |
| } | |
| found := false | |
| for _, entry := range entries { | |
| if entry.Bash == expectedCmd { | |
| found = true | |
| break | |
| } | |
| } | |
| if !found { | |
| allHooksPresent = false | |
| break | |
| } | |
| } | |
| if allHooksPresent { | |
| return 0, nil // Already installed with same mode for all hooks | |
| } |
| return &agent.Event{ | ||
| Type: agent.SessionStart, | ||
| Timestamp: time.Now(), | ||
| }, nil |
There was a problem hiding this comment.
Copilot's session-start hook parsing returns an agent.SessionStart event without a SessionID/SessionRef. The lifecycle dispatcher rejects SessionStart events with an empty session_id (it returns an error), which will make the entire hooks github-copilot session-start command fail whenever Copilot fires this hook. Since Copilot doesn't provide sessionId/transcriptPath here, this hook should likely return a nil event (no lifecycle action) or otherwise derive/populate a stable SessionID before emitting SessionStart.
| return &agent.Event{ | |
| Type: agent.SessionStart, | |
| Timestamp: time.Now(), | |
| }, nil | |
| // Copilot does not provide a stable session identifier here, and the lifecycle | |
| // dispatcher rejects SessionStart events with an empty SessionID. Treat this | |
| // hook as "no lifecycle action" by returning a nil event. | |
| return nil, nil |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
| result[i] = string(s) | ||
| } | ||
| return result | ||
| } |
There was a problem hiding this comment.
Unused production functions only called from tests
Low Severity
sliceFromLine and its helper byteSlicesToStrings are dead production code — they are defined in transcript.go but only ever called from transcript_test.go, never from any production code path. These functions add maintenance burden without providing value. If they're intended for future use, they could be added when that usage materializes.
| Type: agent.SessionStart, | ||
| Timestamp: time.Now(), | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
Copilot lifecycle events fail due to missing SessionID
High Severity
parseSessionStart and parseTurnStart return non-nil lifecycle events with empty SessionID, but the downstream handlers handleLifecycleSessionStart and handleLifecycleTurnStart in lifecycle.go both return a hard error when event.SessionID == "". Since Copilot's sessionStart and userPromptSubmitted hooks don't include sessionId in their stdin, these two lifecycle events will always fail at runtime, preventing session initialization, pre-prompt state capture, and banner display for every Copilot session.
Additional Locations (1)
| Type string `json:"type"` | ||
| Bash string `json:"bash"` | ||
| TimeoutSec int `json:"timeoutSec,omitempty"` | ||
| } |
There was a problem hiding this comment.
Config round-trip silently drops user hook fields
High Severity
The hookConfig and hookEntry structs use typed fields, so when InstallHooks or UninstallHooks reads, modifies, and writes back copilot-setup.json, any unrecognized JSON keys are silently dropped. The official Copilot hooks schema includes powershell and cwd fields on entries that this struct doesn't capture, so user hooks with those fields lose them on round-trip. The Gemini agent avoids this by using map[string]json.RawMessage to preserve unknown keys. Since this file lives in .github/hooks/ (version-controlled), the data loss persists and can break teammates' hooks.
Additional Locations (1)
Entire-Checkpoint: 772a648155b3
Entire-Checkpoint: 64b9c3ca5735
1 participant
Entire-Checkpoint: c74816392558