diff --git a/backend/.golangci.yml b/backend/.golangci.yml index 8cc8082..583e5cb 100644 --- a/backend/.golangci.yml +++ b/backend/.golangci.yml @@ -86,6 +86,7 @@ linters: - G104 # unchecked errors — errcheck owns this - G304 # file inclusion via variable — paths are config/run-file/worktree-derived, not user input - G703 # path traversal via taint analysis — same as G304: binary-resolution and worktree-derived paths, not user input + - G704 # SSRF via taint analysis — the daemon client's host is hardcoded loopback; only the request path varies, so it cannot be steered to an external host exclusions: generated: lax # skip sqlc/codegen ("Code generated ... DO NOT EDIT") diff --git a/backend/go.mod b/backend/go.mod index 70689d8..76e1cbb 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -6,8 +6,10 @@ require ( github.com/coder/websocket v1.8.14 github.com/creack/pty v1.1.24 github.com/go-chi/chi/v5 v5.1.0 + github.com/google/uuid v1.6.0 github.com/pressly/goose/v3 v3.27.1 github.com/spf13/cobra v1.10.1 + github.com/spf13/pflag v1.0.9 github.com/swaggest/jsonschema-go v0.3.79 github.com/swaggest/openapi-go v0.2.61 golang.org/x/sys v0.43.0 @@ -17,17 +19,16 @@ require ( require ( github.com/dustin/go-humanize v1.0.1 // indirect - github.com/google/uuid v1.6.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/mattn/go-isatty v0.0.21 // indirect github.com/mfridman/interpolate v0.0.2 // indirect github.com/ncruces/go-strftime v1.0.0 // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/sethvargo/go-retry v0.3.0 // indirect - github.com/spf13/pflag v1.0.9 // indirect github.com/swaggest/refl v1.4.0 // indirect go.uber.org/multierr v1.11.0 // indirect golang.org/x/sync v0.20.0 // indirect + golang.org/x/tools v0.43.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect modernc.org/libc v1.72.3 // indirect modernc.org/mathutil v1.7.1 // indirect diff --git a/backend/go.sum b/backend/go.sum index 718a4a1..83ac372 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -60,14 +60,14 @@ github.com/yudai/golcs v0.0.0-20170316035057-ecda9a501e82 h1:BHyfKlQyqbsFN5p3Ifn github.com/yudai/golcs v0.0.0-20170316035057-ecda9a501e82/go.mod h1:lgjkn3NuSvDfVJdfcVVdX+jpBxNmX4rDAzaS45IcYoM= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= -golang.org/x/mod v0.33.0 h1:tHFzIWbBifEmbwtGz65eaWyGiGZatSrT9prnU8DbVL8= -golang.org/x/mod v0.33.0/go.mod h1:swjeQEj+6r7fODbD2cqrnje9PnziFuw4bmLbBZFrQ5w= +golang.org/x/mod v0.34.0 h1:xIHgNUUnW6sYkcM5Jleh05DvLOtwc6RitGHbDk4akRI= +golang.org/x/mod v0.34.0/go.mod h1:ykgH52iCZe79kzLLMhyCUzhMci+nQj+0XkbXpNYtVjY= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k= -golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0= +golang.org/x/tools v0.43.0 h1:12BdW9CeB3Z+J/I/wj34VMl8X+fEXBxVR90JeMX5E7s= +golang.org/x/tools v0.43.0/go.mod h1:uHkMso649BX2cZK6+RpuIPXS3ho2hZo4FVwfoy1vIk0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= diff --git a/backend/internal/adapters/agent/activitydispatch/dispatch.go b/backend/internal/adapters/agent/activitydispatch/dispatch.go index b163ec8..775d65b 100644 --- a/backend/internal/adapters/agent/activitydispatch/dispatch.go +++ b/backend/internal/adapters/agent/activitydispatch/dispatch.go @@ -1,6 +1,11 @@ // Package activitydispatch is the single source of truth mapping the agent // token in `ao hooks ` onto the function that interprets that // agent's hook callbacks as an AO activity state. +// +// The hidden `ao hooks` CLI command dispatches a live callback through it. Every +// adapter that installs `ao hooks ` callbacks must have a deriver +// registered here — otherwise the adapter writes callbacks that nothing on the +// receiving side understands, so its activity is silently never reported. package activitydispatch import ( @@ -15,6 +20,7 @@ import ( type DeriveFunc func(event string, payload []byte) (domain.ActivityState, bool) // Derivers maps the agent token in `ao hooks ` to its deriver. +// Per-adapter PRs add their tokens here as they land. var Derivers = map[string]DeriveFunc{ "claude-code": claudecode.DeriveActivityState, "codex": codex.DeriveActivityState, @@ -22,7 +28,8 @@ var Derivers = map[string]DeriveFunc{ } // Derive looks up the deriver for an agent token and applies it. ok=false when -// the token has no registered deriver or the event carries no activity signal. +// the token has no registered deriver or the event carries no activity signal — +// the caller reports nothing in either case. func Derive(agent, event string, payload []byte) (domain.ActivityState, bool) { derive, found := Derivers[agent] if !found { diff --git a/backend/internal/adapters/agent/claudecode/activity.go b/backend/internal/adapters/agent/claudecode/activity.go index a4b2400..a0e4911 100644 --- a/backend/internal/adapters/agent/claudecode/activity.go +++ b/backend/internal/adapters/agent/claudecode/activity.go @@ -6,14 +6,23 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/domain" ) -// DeriveActivityState maps a Claude Code hook event and its native stdin payload -// onto an AO activity state. The bool is false when the event carries no -// activity signal. +// DeriveActivityState maps a Claude Code hook event (and its native stdin +// payload) onto an AO activity state. The bool is false when the event carries +// no activity signal — e.g. SessionStart (metadata only, v1), a Notification +// type we don't track, or a SessionEnd reason that doesn't actually end the AO +// session — in which case the caller reports nothing. +// +// event is the AO hook sub-command name installed in claudeManagedHooks +// ("user-prompt-submit", "stop", "notification", "session-end", ...), NOT the +// native Claude event name. Keeping this beside hooks.go means the events AO +// installs and what they mean live in one place. func DeriveActivityState(event string, payload []byte) (domain.ActivityState, bool) { switch event { case "user-prompt-submit": return domain.ActivityActive, true case "stop": + // End of a turn: the agent is idle but alive (not exited). A following + // Notification(idle_prompt) upgrades this to the sticky waiting_input. return domain.ActivityIdle, true case "notification": return notificationState(payload) @@ -24,6 +33,10 @@ func DeriveActivityState(event string, payload []byte) (domain.ActivityState, bo } } +// notificationState reports waiting_input only for the notification types that +// mean "the agent is blocked on the user": a pending tool-permission prompt or +// an idle prompt awaiting the next instruction. Other types (auth_success, +// elicitation_*) carry no activity meaning, as does a malformed payload. func notificationState(payload []byte) (domain.ActivityState, bool) { var p struct { NotificationType string `json:"notification_type"` @@ -37,6 +50,13 @@ func notificationState(payload []byte) (domain.ActivityState, bool) { } } +// sessionEndState reports exited for reasons that actually end the session. +// clear/resume keep the same AO session alive (a new native session continues +// in the worktree), so they report nothing. Any other reason — logout, +// prompt_input_exit, bypass_permissions_disabled, other, or an absent/unknown +// reason on a SessionEnd that did fire — is treated as a real exit. SessionEnd +// is not guaranteed on crash/SIGKILL, so the reaper remains the backstop; both +// paths guard on IsTerminated, so whichever lands first wins. func sessionEndState(payload []byte) (domain.ActivityState, bool) { var p struct { Reason string `json:"reason"` diff --git a/backend/internal/adapters/agent/claudecode/activity_test.go b/backend/internal/adapters/agent/claudecode/activity_test.go index bf25a05..c503dc6 100644 --- a/backend/internal/adapters/agent/claudecode/activity_test.go +++ b/backend/internal/adapters/agent/claudecode/activity_test.go @@ -19,9 +19,11 @@ func TestDeriveActivityState(t *testing.T) { {"notification idle_prompt -> waiting_input", "notification", `{"notification_type":"idle_prompt"}`, domain.ActivityWaitingInput, true}, {"notification permission_prompt -> waiting_input", "notification", `{"notification_type":"permission_prompt"}`, domain.ActivityWaitingInput, true}, {"notification auth_success -> no signal", "notification", `{"notification_type":"auth_success"}`, "", false}, + {"notification empty type -> no signal", "notification", `{}`, "", false}, {"notification malformed payload -> no signal", "notification", `not json`, "", false}, {"session-end logout -> exited", "session-end", `{"reason":"logout"}`, domain.ActivityExited, true}, {"session-end prompt_input_exit -> exited", "session-end", `{"reason":"prompt_input_exit"}`, domain.ActivityExited, true}, + {"session-end other -> exited", "session-end", `{"reason":"other"}`, domain.ActivityExited, true}, {"session-end absent reason -> exited", "session-end", `{}`, domain.ActivityExited, true}, {"session-end clear -> no signal", "session-end", `{"reason":"clear"}`, "", false}, {"session-end resume -> no signal", "session-end", `{"reason":"resume"}`, "", false}, diff --git a/backend/internal/adapters/agent/claudecode/claudecode.go b/backend/internal/adapters/agent/claudecode/claudecode.go index 0c94359..8351077 100644 --- a/backend/internal/adapters/agent/claudecode/claudecode.go +++ b/backend/internal/adapters/agent/claudecode/claudecode.go @@ -37,14 +37,6 @@ const ( // adapterID is the registry id and the value users pass to // `ao spawn --agent`. adapterID = "claude-code" - - // Normalized session-metadata keys the Claude Code hooks persist into the - // AO session store and SessionInfo reads back. Shared vocabulary - // with the Codex adapter so the dashboard treats every agent uniformly. - // The native session id key lives in ports as MetadataKeyAgentSessionID - // because the Session Manager also reads it. - claudeTitleMetadataKey = "title" - claudeSummaryMetadataKey = "summary" ) // claudeSessionNamespace seeds the UUIDv5 derivation that maps an AO @@ -220,8 +212,8 @@ func (p *Plugin) SessionInfo(ctx context.Context, session ports.SessionRef) (por } info := ports.SessionInfo{ AgentSessionID: session.Metadata[ports.MetadataKeyAgentSessionID], - Title: session.Metadata[claudeTitleMetadataKey], - Summary: session.Metadata[claudeSummaryMetadataKey], + Title: session.Metadata[ports.MetadataKeyTitle], + Summary: session.Metadata[ports.MetadataKeySummary], } if info.AgentSessionID == "" && info.Title == "" && info.Summary == "" { return ports.SessionInfo{}, false, nil diff --git a/backend/internal/adapters/agent/claudecode/claudecode_test.go b/backend/internal/adapters/agent/claudecode/claudecode_test.go index c512d79..bdaf346 100644 --- a/backend/internal/adapters/agent/claudecode/claudecode_test.go +++ b/backend/internal/adapters/agent/claudecode/claudecode_test.go @@ -214,8 +214,8 @@ func TestGetAgentHooksInstallsClaudeHooks(t *testing.T) { if m := matcherForCommand(config.Hooks["UserPromptSubmit"], "ao hooks claude-code user-prompt-submit"); m != nil { t.Fatalf("UserPromptSubmit matcher = %v, want none", m) } - // Notification and SessionEnd install with no matcher; the handler filters - // on the payload. + // Notification and SessionEnd install with no matcher (they fire for all + // sub-types; the handler filters on the payload). if m := matcherForCommand(config.Hooks["Notification"], "ao hooks claude-code notification"); m != nil { t.Fatalf("Notification matcher = %v, want none", m) } @@ -303,8 +303,8 @@ func TestSessionInfoReadsHookMetadata(t *testing.T) { WorkspacePath: "/some/path", Metadata: map[string]string{ ports.MetadataKeyAgentSessionID: "claude-native-1", - claudeTitleMetadataKey: "Fix login redirect", - claudeSummaryMetadataKey: "Updated the auth callback and tests.", + ports.MetadataKeyTitle: "Fix login redirect", + ports.MetadataKeySummary: "Updated the auth callback and tests.", "ignored": "not returned", }, }) diff --git a/backend/internal/adapters/agent/claudecode/hooks.go b/backend/internal/adapters/agent/claudecode/hooks.go index 8f43aca..0f9bd92 100644 --- a/backend/internal/adapters/agent/claudecode/hooks.go +++ b/backend/internal/adapters/agent/claudecode/hooks.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strings" + "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/hookutil" "github.com/aoagents/agent-orchestrator/backend/internal/ports" ) @@ -56,7 +57,8 @@ var claudeStartupMatcher = "startup" // activity-state signals back into AO's store (see DeriveActivityState). // Notification and SessionEnd carry no matcher: each installs once and fires // for every sub-type, and the handler filters on the payload's -// notification_type / reason field. +// notification_type / reason field — installing one command under multiple +// matchers would trip the per-command dedup in claudeHookCommandExists. var claudeManagedHooks = []claudeHookSpec{ {Event: "SessionStart", Matcher: &claudeStartupMatcher, Command: claudeHookCommandPrefix + "session-start"}, {Event: "UserPromptSubmit", Command: claudeHookCommandPrefix + "user-prompt-submit"}, @@ -233,36 +235,12 @@ func writeClaudeSettings(settingsPath string, topLevel, rawHooks map[string]json return fmt.Errorf("encode %s: %w", settingsPath, err) } data = append(data, '\n') - if err := atomicWriteFile(settingsPath, data, 0o600); err != nil { + if err := hookutil.AtomicWriteFile(settingsPath, data, 0o600); err != nil { return fmt.Errorf("write %s: %w", settingsPath, err) } return nil } -// atomicWriteFile writes data to path via a temp file in the same directory -// followed by a rename, so a crash or signal mid-write can't leave a truncated -// or empty file that Claude Code then fails to parse (silently disabling hooks). -func atomicWriteFile(path string, data []byte, perm os.FileMode) error { - tmp, err := os.CreateTemp(filepath.Dir(path), ".ao-tmp-*") - if err != nil { - return err - } - tmpName := tmp.Name() - defer func() { _ = os.Remove(tmpName) }() // no-op once renamed - if _, err := tmp.Write(data); err != nil { - _ = tmp.Close() - return err - } - if err := tmp.Chmod(perm); err != nil { - _ = tmp.Close() - return err - } - if err := tmp.Close(); err != nil { - return err - } - return os.Rename(tmpName, path) -} - // groupClaudeHooksByEvent groups the managed hook specs by their Claude event so // each event's settings array is rewritten once. func groupClaudeHooksByEvent() map[string][]claudeHookSpec { diff --git a/backend/internal/adapters/agent/codex/activity.go b/backend/internal/adapters/agent/codex/activity.go index 6b08cb3..d934201 100644 --- a/backend/internal/adapters/agent/codex/activity.go +++ b/backend/internal/adapters/agent/codex/activity.go @@ -4,6 +4,11 @@ import "github.com/aoagents/agent-orchestrator/backend/internal/domain" // DeriveActivityState maps a Codex hook event onto an AO activity state. The // bool is false when the event carries no activity signal. +// +// event is the AO hook sub-command name installed in codexManagedHooks +// ("user-prompt-submit", "permission-request", "stop", ...), not the native +// Codex event name. Codex currently has no SessionEnd/Notification equivalent +// in the adapter, so runtime exit still falls back to the reaper. func DeriveActivityState(event string, _ []byte) (domain.ActivityState, bool) { switch event { case "user-prompt-submit": diff --git a/backend/internal/adapters/agent/codex/codex.go b/backend/internal/adapters/agent/codex/codex.go index a9a32a1..bc14408 100644 --- a/backend/internal/adapters/agent/codex/codex.go +++ b/backend/internal/adapters/agent/codex/codex.go @@ -19,11 +19,6 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/ports" ) -const ( - codexTitleMetadataKey = "title" - codexSummaryMetadataKey = "summary" -) - // Plugin is the Codex agent adapter. It is safe for concurrent use; the binary // path is resolved once and cached under binaryMu. type Plugin struct { @@ -132,8 +127,8 @@ func (p *Plugin) SessionInfo(ctx context.Context, session ports.SessionRef) (por } info := ports.SessionInfo{ AgentSessionID: session.Metadata[ports.MetadataKeyAgentSessionID], - Title: session.Metadata[codexTitleMetadataKey], - Summary: session.Metadata[codexSummaryMetadataKey], + Title: session.Metadata[ports.MetadataKeyTitle], + Summary: session.Metadata[ports.MetadataKeySummary], } if info.AgentSessionID == "" && info.Title == "" && info.Summary == "" { return ports.SessionInfo{}, false, nil diff --git a/backend/internal/adapters/agent/codex/codex_test.go b/backend/internal/adapters/agent/codex/codex_test.go index 3d93c9d..63757e3 100644 --- a/backend/internal/adapters/agent/codex/codex_test.go +++ b/backend/internal/adapters/agent/codex/codex_test.go @@ -298,8 +298,8 @@ func TestSessionInfoReadsHookMetadata(t *testing.T) { WorkspacePath: "/some/path", Metadata: map[string]string{ ports.MetadataKeyAgentSessionID: "thread-123", - codexTitleMetadataKey: "Fix login redirect", - codexSummaryMetadataKey: "Updated the auth callback and tests.", + ports.MetadataKeyTitle: "Fix login redirect", + ports.MetadataKeySummary: "Updated the auth callback and tests.", "ignored": "not returned", }, }) diff --git a/backend/internal/adapters/agent/codex/hooks.go b/backend/internal/adapters/agent/codex/hooks.go index 5470b7b..88c2cef 100644 --- a/backend/internal/adapters/agent/codex/hooks.go +++ b/backend/internal/adapters/agent/codex/hooks.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strings" + "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/hookutil" "github.com/aoagents/agent-orchestrator/backend/internal/ports" ) @@ -229,35 +230,12 @@ func writeCodexHooks(hooksPath string, topLevel, rawHooks map[string]json.RawMes return fmt.Errorf("encode %s: %w", hooksPath, err) } data = append(data, '\n') - if err := atomicWriteFile(hooksPath, data, 0o600); err != nil { + if err := hookutil.AtomicWriteFile(hooksPath, data, 0o600); err != nil { return fmt.Errorf("write %s: %w", hooksPath, err) } return nil } -// atomicWriteFile writes data to path via a temp file + rename, so a crash mid- -// write can't leave a truncated/empty file that Codex then fails to parse. -func atomicWriteFile(path string, data []byte, perm os.FileMode) error { - tmp, err := os.CreateTemp(filepath.Dir(path), ".ao-tmp-*") - if err != nil { - return err - } - tmpName := tmp.Name() - defer func() { _ = os.Remove(tmpName) }() - if _, err := tmp.Write(data); err != nil { - _ = tmp.Close() - return err - } - if err := tmp.Chmod(perm); err != nil { - _ = tmp.Close() - return err - } - if err := tmp.Close(); err != nil { - return err - } - return os.Rename(tmpName, path) -} - // groupCodexHooksByEvent groups the managed hook specs by their Codex event so // each event's array is rewritten once. func groupCodexHooksByEvent() map[string][]codexHookSpec { @@ -382,7 +360,7 @@ func ensureCodexHooksFeatureEnabled(workspacePath string) error { if err := os.MkdirAll(filepath.Dir(configPath), 0o750); err != nil { return fmt.Errorf("create .codex directory: %w", err) } - if err := atomicWriteFile(configPath, []byte(content), 0o600); err != nil { + if err := hookutil.AtomicWriteFile(configPath, []byte(content), 0o600); err != nil { return fmt.Errorf("write config.toml: %w", err) } return nil diff --git a/backend/internal/adapters/agent/hookutil/hookutil.go b/backend/internal/adapters/agent/hookutil/hookutil.go new file mode 100644 index 0000000..4333577 --- /dev/null +++ b/backend/internal/adapters/agent/hookutil/hookutil.go @@ -0,0 +1,37 @@ +// Package hookutil holds small filesystem helpers shared by the agent hook +// installers (claude-code, codex, opencode). It centralizes the atomic-write +// primitive so every adapter writes hook config the same crash-safe way. +package hookutil + +import ( + "os" + "path/filepath" +) + +// AtomicWriteFile writes data to path via a temp file in the same directory +// followed by a rename, so a crash or signal mid-write can't leave a truncated +// or empty file that the agent then fails to parse (silently disabling hooks). +func AtomicWriteFile(path string, data []byte, perm os.FileMode) error { + tmp, err := os.CreateTemp(filepath.Dir(path), ".ao-tmp-*") + if err != nil { + return err + } + tmpName := tmp.Name() + defer func() { _ = os.Remove(tmpName) }() // no-op once renamed + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Chmod(perm); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Sync(); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + return os.Rename(tmpName, path) +} diff --git a/backend/internal/adapters/agent/opencode/activity.go b/backend/internal/adapters/agent/opencode/activity.go index e9ad7d3..1691359 100644 --- a/backend/internal/adapters/agent/opencode/activity.go +++ b/backend/internal/adapters/agent/opencode/activity.go @@ -3,7 +3,10 @@ package opencode import "github.com/aoagents/agent-orchestrator/backend/internal/domain" // DeriveActivityState maps an opencode plugin hook event onto an AO activity -// state. The bool is false when the event carries no activity signal. +// state. The opencode plugin (assets/ao-activity.ts) normalizes opencode's +// native events to "session-start" / "user-prompt-submit" / "stop" before +// invoking `ao hooks opencode `. The bool is false when the event +// carries no activity signal. func DeriveActivityState(event string, _ []byte) (domain.ActivityState, bool) { switch event { case "session-start": diff --git a/backend/internal/adapters/agent/opencode/hooks.go b/backend/internal/adapters/agent/opencode/hooks.go index 8e7b1b5..6f87dc4 100644 --- a/backend/internal/adapters/agent/opencode/hooks.go +++ b/backend/internal/adapters/agent/opencode/hooks.go @@ -10,6 +10,7 @@ import ( _ "embed" + "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/hookutil" "github.com/aoagents/agent-orchestrator/backend/internal/ports" ) @@ -90,7 +91,7 @@ func (p *Plugin) GetAgentHooks(ctx context.Context, cfg ports.WorkspaceHookConfi if err := os.MkdirAll(filepath.Dir(pluginPath), 0o750); err != nil { return fmt.Errorf("opencode.GetAgentHooks: create plugin dir: %w", err) } - if err := atomicWriteFile(pluginPath, []byte(opencodePluginSource), 0o600); err != nil { + if err := hookutil.AtomicWriteFile(pluginPath, []byte(opencodePluginSource), 0o600); err != nil { return fmt.Errorf("opencode.GetAgentHooks: write plugin: %w", err) } return nil @@ -155,31 +156,3 @@ func isAOManagedPlugin(path string) (bool, error) { } return strings.Contains(string(data), opencodePluginSentinel), nil } - -// atomicWriteFile writes data to path via a temp file + rename, so a crash mid- -// write can't leave a truncated plugin file that opencode then fails to import -// (silently disabling activity reporting). -func atomicWriteFile(path string, data []byte, perm os.FileMode) error { - tmp, err := os.CreateTemp(filepath.Dir(path), ".ao-tmp-*") - if err != nil { - return err - } - tmpName := tmp.Name() - defer func() { _ = os.Remove(tmpName) }() // no-op once renamed - if _, err := tmp.Write(data); err != nil { - _ = tmp.Close() - return err - } - if err := tmp.Chmod(perm); err != nil { - _ = tmp.Close() - return err - } - if err := tmp.Sync(); err != nil { - _ = tmp.Close() - return err - } - if err := tmp.Close(); err != nil { - return err - } - return os.Rename(tmpName, path) -} diff --git a/backend/internal/adapters/agent/registry/registry.go b/backend/internal/adapters/agent/registry/registry.go new file mode 100644 index 0000000..317cd77 --- /dev/null +++ b/backend/internal/adapters/agent/registry/registry.go @@ -0,0 +1,67 @@ +// Package registry is the single source of truth for the agent adapters the +// daemon ships. The daemon wires sessions through it, so adding a harness is a +// single edit to Constructors rather than a list maintained in several places. +package registry + +import ( + "fmt" + + "github.com/aoagents/agent-orchestrator/backend/internal/adapters" + "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/claudecode" + "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/codex" + "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/opencode" + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/ports" +) + +// Constructors returns a fresh instance of every agent adapter the daemon +// ships, in a stable registration order. Adding a new harness means adding its +// constructor here (and a domain.AgentHarness constant) — the one edit the +// daemon picks up. +func Constructors() []adapters.Adapter { + return []adapters.Adapter{ + claudecode.New(), + codex.New(), + opencode.New(), + } +} + +// Build returns a registry populated with the shipped agent adapters, keyed by +// manifest id. Registration only fails on an empty/duplicate id — a programmer +// error, not a runtime condition. +func Build() (*adapters.Registry, error) { + reg := adapters.NewRegistry() + for _, a := range Constructors() { + if err := reg.Register(a); err != nil { + return nil, fmt.Errorf("register agent adapter %q: %w", a.Manifest().ID, err) + } + } + return reg, nil +} + +// HarnessAgent pairs a session harness with the adapter that drives it. The +// harness is the adapter's manifest id, which is also the domain.AgentHarness +// value a session carries and the `--harness` flag users pass. +type HarnessAgent struct { + Harness domain.AgentHarness + Agent ports.Agent +} + +// Harnessed returns every shipped adapter that drives an agent, paired with its +// harness, in Constructors() order. An adapter that does not implement +// ports.Agent is skipped. +func Harnessed() []HarnessAgent { + cons := Constructors() + out := make([]HarnessAgent, 0, len(cons)) + for _, a := range cons { + agent, ok := a.(ports.Agent) + if !ok { + continue + } + out = append(out, HarnessAgent{ + Harness: domain.AgentHarness(a.Manifest().ID), + Agent: agent, + }) + } + return out +} diff --git a/backend/internal/cli/hooks.go b/backend/internal/cli/hooks.go index 302b274..a5a991c 100644 --- a/backend/internal/cli/hooks.go +++ b/backend/internal/cli/hooks.go @@ -6,6 +6,7 @@ import ( "io" "net/url" "os" + "regexp" "strings" "github.com/spf13/cobra" @@ -13,6 +14,11 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/activitydispatch" ) +// sessionIDPattern bounds the AO_SESSION_ID we will place in a request path to +// the id alphabet the daemon issues. Validating the externally-set env value +// before it reaches the loopback URL keeps it from steering the request. +var sessionIDPattern = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + // setActivityAPIRequest mirrors the daemon's SetActivityRequest body for // POST /api/v1/sessions/{id}/activity. The CLI keeps its own copy so it need // not import httpd. @@ -42,9 +48,10 @@ func newHooksCommand(ctx *commandContext) *cobra.Command { func (c *commandContext) runHook(ctx context.Context, agent, event string) error { sessionID := strings.TrimSpace(os.Getenv("AO_SESSION_ID")) - if sessionID == "" { - // Not an AO-managed session. Return before reading stdin so a manual - // invocation without a piped payload can't block on EOF. + if !sessionIDPattern.MatchString(sessionID) { + // Not an AO-managed session (unset/empty), or an id we won't put in a + // request path. Return before reading stdin so a manual invocation + // without a piped payload can't block on EOF. return nil } payload, err := io.ReadAll(c.deps.In) diff --git a/backend/internal/cli/hooks_test.go b/backend/internal/cli/hooks_test.go index 45f571e..57fb90a 100644 --- a/backend/internal/cli/hooks_test.go +++ b/backend/internal/cli/hooks_test.go @@ -144,6 +144,24 @@ func TestHooks_OpenCodeUserPromptReportsActive(t *testing.T) { } } +func TestHooks_RejectsMalformedSessionID(t *testing.T) { + t.Setenv("AO_SESSION_ID", "../etc/passwd") + cfg := setConfigEnv(t) + srv, capture := activityServer(t, http.StatusOK, `{}`) + writeRunFileFor(t, cfg, srv) + + _, _, err := executeCLI(t, Deps{ + In: strings.NewReader(`{"reason":"logout"}`), + ProcessAlive: func(int) bool { return true }, + }, "hooks", "claude-code", "session-end") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if capture.hits != 0 { + t.Errorf("expected no daemon call for an out-of-alphabet session id, got %d", capture.hits) + } +} + func TestHooks_NoSessionIDIsNoOp(t *testing.T) { t.Setenv("AO_SESSION_ID", "") cfg := setConfigEnv(t) diff --git a/backend/internal/cli/spawn.go b/backend/internal/cli/spawn.go index 30cc7fa..7f09cd3 100644 --- a/backend/internal/cli/spawn.go +++ b/backend/internal/cli/spawn.go @@ -6,6 +6,7 @@ import ( "net/url" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/aoagents/agent-orchestrator/backend/internal/adapters/runtime/zellij" ) @@ -113,8 +114,16 @@ func newSpawnCommand(ctx *commandContext) *cobra.Command { }, } f := cmd.Flags() + // --agent is an alias for --harness so the more intuitive `ao spawn --agent + // droid` works identically; both resolve to the same harness flag. + f.SetNormalizeFunc(func(_ *pflag.FlagSet, name string) pflag.NormalizedName { + if name == "agent" { + name = "harness" + } + return pflag.NormalizedName(name) + }) f.StringVar(&opts.project, "project", "", "Project id to spawn the session in (required)") - f.StringVar(&opts.harness, "harness", "", "Agent harness: claude-code, codex, … (default: the daemon's AO_AGENT)") + f.StringVar(&opts.harness, "harness", "", "Agent harness / --agent: claude-code, codex, aider, opencode, grok, droid, amp, agy, crush, cursor, qwen, copilot, goose, auggie, continue, devin, cline, kimi, kiro, kilocode, vibe, pi, autohand (default: the daemon's AO_AGENT)") f.StringVar(&opts.branch, "branch", "", "Branch for the session worktree (default: ao/)") f.StringVar(&opts.prompt, "prompt", "", "Initial prompt for the agent") f.StringVar(&opts.issue, "issue", "", "Issue id to associate with the session") diff --git a/backend/internal/daemon/lifecycle_wiring.go b/backend/internal/daemon/lifecycle_wiring.go index d978fc6..47251f2 100644 --- a/backend/internal/daemon/lifecycle_wiring.go +++ b/backend/internal/daemon/lifecycle_wiring.go @@ -7,9 +7,7 @@ import ( "path/filepath" "github.com/aoagents/agent-orchestrator/backend/internal/adapters" - "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/claudecode" - "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/codex" - "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/opencode" + agentregistry "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/registry" "github.com/aoagents/agent-orchestrator/backend/internal/adapters/workspace/gitworktree" "github.com/aoagents/agent-orchestrator/backend/internal/config" "github.com/aoagents/agent-orchestrator/backend/internal/domain" @@ -129,14 +127,10 @@ func newSessionMessenger(store *sqlite.Store, runtime runtimeMessageSender, _ *s // buildAgentRegistry returns a registry populated with the agent adapters the // daemon ships, keyed by manifest id. Registration only fails on an // empty/duplicate id — a programmer error, not a runtime condition. +// The shipped adapter list lives in the adapters/agent/registry package +// (registry.Constructors). Adding a new harness is a one-line edit there. func buildAgentRegistry() (*adapters.Registry, error) { - reg := adapters.NewRegistry() - for _, a := range []adapters.Adapter{claudecode.New(), codex.New(), opencode.New()} { - if err := reg.Register(a); err != nil { - return nil, fmt.Errorf("register agent adapter %q: %w", a.Manifest().ID, err) - } - } - return reg, nil + return agentregistry.Build() } // agentRegistry adapts the generic adapter Registry to ports.AgentResolver: it diff --git a/backend/internal/domain/activity.go b/backend/internal/domain/activity.go index 468ee9e..f4a5126 100644 --- a/backend/internal/domain/activity.go +++ b/backend/internal/domain/activity.go @@ -2,7 +2,8 @@ package domain import "time" -// ActivityState is how busy the agent is, derived from its output/JSONL. +// ActivityState is how busy the agent is, reported via the agent's CLI hook +// callbacks (see docs/agent/README.md), not inferred from transcript/JSONL type ActivityState string // Activity states. WaitingInput is sticky (see IsSticky). diff --git a/backend/internal/domain/harness.go b/backend/internal/domain/harness.go index 90d8617..41a8474 100644 --- a/backend/internal/domain/harness.go +++ b/backend/internal/domain/harness.go @@ -9,4 +9,23 @@ const ( HarnessCodex AgentHarness = "codex" HarnessAider AgentHarness = "aider" HarnessOpenCode AgentHarness = "opencode" + HarnessGrok AgentHarness = "grok" + HarnessDroid AgentHarness = "droid" + HarnessAmp AgentHarness = "amp" + HarnessAgy AgentHarness = "agy" + HarnessCrush AgentHarness = "crush" + HarnessCursor AgentHarness = "cursor" + HarnessQwen AgentHarness = "qwen" + HarnessCopilot AgentHarness = "copilot" + HarnessGoose AgentHarness = "goose" + HarnessAuggie AgentHarness = "auggie" + HarnessContinue AgentHarness = "continue" + HarnessDevin AgentHarness = "devin" + HarnessCline AgentHarness = "cline" + HarnessKimi AgentHarness = "kimi" + HarnessKiro AgentHarness = "kiro" + HarnessKilocode AgentHarness = "kilocode" + HarnessVibe AgentHarness = "vibe" + HarnessPi AgentHarness = "pi" + HarnessAutohand AgentHarness = "autohand" ) diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index 7a5c6f5..f2f1767 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -1332,6 +1332,25 @@ components: - codex - aider - opencode + - grok + - droid + - amp + - agy + - crush + - cursor + - qwen + - copilot + - goose + - auggie + - continue + - devin + - cline + - kimi + - kiro + - kilocode + - vibe + - pi + - autohand type: string issueId: type: string diff --git a/backend/internal/httpd/controllers/dto.go b/backend/internal/httpd/controllers/dto.go index f0fcbe9..45ad38b 100644 --- a/backend/internal/httpd/controllers/dto.go +++ b/backend/internal/httpd/controllers/dto.go @@ -121,7 +121,7 @@ type SpawnSessionRequest struct { ProjectID domain.ProjectID `json:"projectId"` IssueID domain.IssueID `json:"issueId,omitempty"` Kind domain.SessionKind `json:"kind,omitempty" enum:"worker,orchestrator"` - Harness domain.AgentHarness `json:"harness,omitempty" enum:"claude-code,codex,aider,opencode"` + Harness domain.AgentHarness `json:"harness,omitempty" enum:"claude-code,codex,aider,opencode,grok,droid,amp,agy,crush,cursor,qwen,copilot,goose,auggie,continue,devin,cline,kimi,kiro,kilocode,vibe,pi,autohand"` Branch string `json:"branch,omitempty"` Prompt string `json:"prompt,omitempty" maxLength:"4096"` AgentRules string `json:"agentRules,omitempty"` diff --git a/backend/internal/httpd/controllers/sessions.go b/backend/internal/httpd/controllers/sessions.go index 50a0454..e828d9f 100644 --- a/backend/internal/httpd/controllers/sessions.go +++ b/backend/internal/httpd/controllers/sessions.go @@ -37,7 +37,11 @@ type SessionService interface { ClaimPR(ctx context.Context, id domain.SessionID, ref string, opts sessionsvc.ClaimPROptions) (sessionsvc.ClaimPRResult, error) } -// ActivityRecorder applies an agent activity-state signal to a session. +// ActivityRecorder applies an agent activity-state signal to a session. It is +// satisfied directly by *lifecycle.Manager: an activity signal is a pure +// lifecycle reduction (no runtime/workspace teardown), so it bypasses +// SessionService rather than threading a no-op passthrough through the session +// manager. type ActivityRecorder interface { ApplyActivitySignal(ctx context.Context, id domain.SessionID, s ports.ActivitySignal) error } @@ -253,6 +257,10 @@ func (c *SessionsController) send(w http.ResponseWriter, r *http.Request) { envelope.WriteJSON(w, http.StatusOK, SendSessionMessageResponse{OK: true, SessionID: sessionID(r), Message: message}) } +// activity records an agent activity-state signal reported by an agent hook +// (via `ao hooks `). It funnels through the single +// lifecycle.Manager so the reaper and hooks never race on the session's +// activity/termination columns. func (c *SessionsController) activity(w http.ResponseWriter, r *http.Request) { if c.Activity == nil { apispec.NotImplemented(w, r, "POST", "/api/v1/sessions/{sessionId}/activity") diff --git a/backend/internal/ports/agent.go b/backend/internal/ports/agent.go index 6cee911..11cfd95 100644 --- a/backend/internal/ports/agent.go +++ b/backend/internal/ports/agent.go @@ -49,6 +49,15 @@ type AgentResolver interface { // either side hard-coding the other's vocabulary. const MetadataKeyAgentSessionID = "agentSessionId" +// MetadataKeyTitle and MetadataKeySummary are the SessionRef.Metadata keys +// carrying a session's human title and one-line summary. They are the shared +// vocabulary every adapter reports under, so the dashboard renders agents +// uniformly. +const ( + MetadataKeyTitle = "title" + MetadataKeySummary = "summary" +) + // AgentConfig holds values loaded from the selected agent's config section. // Agent adapters own validation for their custom keys. type AgentConfig map[string]any diff --git a/backend/internal/storage/sqlite/migrate_test.go b/backend/internal/storage/sqlite/migrate_test.go new file mode 100644 index 0000000..2dd3066 --- /dev/null +++ b/backend/internal/storage/sqlite/migrate_test.go @@ -0,0 +1,68 @@ +package sqlite + +import ( + "database/sql" + "path/filepath" + "strings" + "testing" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +// TestMigrateAllowsEveryShippedHarness guards against the collapsed-migration +// silent-no-op concern: a hand-written replace() that fails to widen the +// sessions.harness CHECK (because the target substring drifted) leaves the +// schema accepting only the original harnesses while migrate() still reports +// success. This test opens a fresh DB, runs the migrations, and asserts the +// live sessions schema admits every harness the domain ships, building the +// expected set from the domain constants so it can't silently drift. +func TestMigrateAllowsEveryShippedHarness(t *testing.T) { + db, err := sql.Open("sqlite", "file:"+filepath.Join(t.TempDir(), "ao.db")+pragmas) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + + if err := migrate(db); err != nil { + t.Fatalf("migrate: %v", err) + } + + var schema string + if err := db.QueryRow( + "SELECT sql FROM sqlite_master WHERE type='table' AND name='sessions'", + ).Scan(&schema); err != nil { + t.Fatalf("read sessions schema: %v", err) + } + + harnesses := []domain.AgentHarness{ + domain.HarnessClaudeCode, + domain.HarnessCodex, + domain.HarnessAider, + domain.HarnessOpenCode, + domain.HarnessGrok, + domain.HarnessDroid, + domain.HarnessAmp, + domain.HarnessAgy, + domain.HarnessCrush, + domain.HarnessCursor, + domain.HarnessQwen, + domain.HarnessCopilot, + domain.HarnessGoose, + domain.HarnessAuggie, + domain.HarnessContinue, + domain.HarnessDevin, + domain.HarnessCline, + domain.HarnessKimi, + domain.HarnessKiro, + domain.HarnessKilocode, + domain.HarnessVibe, + domain.HarnessPi, + domain.HarnessAutohand, + } + + for _, h := range harnesses { + if !strings.Contains(schema, "'"+string(h)+"'") { + t.Errorf("sessions.harness CHECK is missing harness %q — the migration that widens it silently no-opped; schema:\n%s", h, schema) + } + } +} diff --git a/backend/internal/storage/sqlite/migrations/0007_allow_implemented_harnesses.sql b/backend/internal/storage/sqlite/migrations/0007_allow_implemented_harnesses.sql new file mode 100644 index 0000000..36c9d65 --- /dev/null +++ b/backend/internal/storage/sqlite/migrations/0007_allow_implemented_harnesses.sql @@ -0,0 +1,42 @@ +-- Widen the sessions.harness CHECK to allow every agent harness AO ships, in a +-- single step. SQLite cannot ALTER a CHECK, so we surgically rewrite the stored +-- CREATE TABLE text in sqlite_master. writable_schema edits must run outside a +-- transaction, and RESET forces an immediate schema reparse on the connection. +-- +-- New harnesses are added here by extending this list, not by chaining a fresh +-- per-harness migration onto the previous one's exact text. + +-- +goose NO TRANSACTION +-- +goose Up +-- +goose StatementBegin +PRAGMA writable_schema = ON; +-- +goose StatementEnd +-- +goose StatementBegin +UPDATE sqlite_master +SET sql = replace( + sql, + 'CHECK (harness IN ('''', ''claude-code'', ''codex'', ''aider'', ''opencode''))', + 'CHECK (harness IN ('''', ''claude-code'', ''codex'', ''aider'', ''opencode'', ''grok'', ''droid'', ''amp'', ''agy'', ''crush'', ''cursor'', ''qwen'', ''copilot'', ''goose'', ''auggie'', ''continue'', ''devin'', ''cline'', ''kimi'', ''kiro'', ''kilocode'', ''vibe'', ''pi'', ''autohand''))' +) +WHERE type = 'table' AND name = 'sessions'; +-- +goose StatementEnd +-- +goose StatementBegin +PRAGMA writable_schema = RESET; +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +PRAGMA writable_schema = ON; +-- +goose StatementEnd +-- +goose StatementBegin +UPDATE sqlite_master +SET sql = replace( + sql, + 'CHECK (harness IN ('''', ''claude-code'', ''codex'', ''aider'', ''opencode'', ''grok'', ''droid'', ''amp'', ''agy'', ''crush'', ''cursor'', ''qwen'', ''copilot'', ''goose'', ''auggie'', ''continue'', ''devin'', ''cline'', ''kimi'', ''kiro'', ''kilocode'', ''vibe'', ''pi'', ''autohand''))', + 'CHECK (harness IN ('''', ''claude-code'', ''codex'', ''aider'', ''opencode''))' +) +WHERE type = 'table' AND name = 'sessions'; +-- +goose StatementEnd +-- +goose StatementBegin +PRAGMA writable_schema = RESET; +-- +goose StatementEnd diff --git a/frontend/src/api/schema.ts b/frontend/src/api/schema.ts index 764a478..fb22fe1 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -472,7 +472,7 @@ export interface components { agentRules?: string; branch?: string; /** @enum {string} */ - harness?: "claude-code" | "codex" | "aider" | "opencode"; + harness?: "claude-code" | "codex" | "aider" | "opencode" | "grok" | "droid" | "amp" | "agy" | "crush" | "cursor" | "qwen" | "copilot" | "goose" | "auggie" | "continue" | "devin" | "cline" | "kimi" | "kiro" | "kilocode" | "vibe" | "pi" | "autohand"; issueId?: string; /** @enum {string} */ kind?: "worker" | "orchestrator";