diff --git a/.compozy/tasks/network-default/reviews-001/_meta.md b/.compozy/tasks/network-default/reviews-001/_meta.md new file mode 100644 index 000000000..97d05592a --- /dev/null +++ b/.compozy/tasks/network-default/reviews-001/_meta.md @@ -0,0 +1,11 @@ +--- +provider: coderabbit +pr: "57" +round: 1 +created_at: 2026-04-24T00:57:29.033415Z +--- + +## Summary +- Total: 4 +- Resolved: 0 +- Unresolved: 4 diff --git a/.compozy/tasks/network-default/reviews-001/issue_001.md b/.compozy/tasks/network-default/reviews-001/issue_001.md new file mode 100644 index 000000000..58cd62c3e --- /dev/null +++ b/.compozy/tasks/network-default/reviews-001/issue_001.md @@ -0,0 +1,28 @@ +--- +status: resolved +file: internal/api/httpapi/helpers_test.go +line: 421 +severity: nitpick +author: coderabbitai[bot] +provider_ref: review:4151161262,nitpick_hash:50d2a42d9b89 +review_hash: 50d2a42d9b89 +source_review_id: "4151161262" +source_review_submitted_at: "2026-04-21T22:49:44Z" +--- + +# Issue 001: Centralize testConfigWithDisabledNetwork. +## Review Comment + +This helper is now duplicated in `internal/api/httpapi/helpers_test.go`, `internal/api/udsapi/helpers_test.go`, and `internal/api/core/test_helpers_test.go`. Moving it into shared testutil would reduce drift the next time config defaults change. + +As per coding guidelines, `**/*_test.go`: "Check for shared test utilities usage to avoid duplication". + +## Triage + +- Decision: `VALID` +- Notes: + - `testConfigWithDisabledNetwork` is currently duplicated in all three cited test helper files, and each copy does the same `aghconfig.DefaultWithHome(homePaths)` plus `cfg.Network.Enabled = false` mutation. + - Root cause: the network-default test additions reused a local helper pattern instead of extracting the shared API-test fixture into `internal/api/testutil`. + - Fix approach: add one shared helper in `internal/api/testutil` and update the three test helper files to call it. This requires minimal out-of-scope edits to `internal/api/udsapi/helpers_test.go`, `internal/api/core/test_helpers_test.go`, and `internal/api/testutil/apitest.go` so the duplication is actually removed rather than moved. + - Implemented: `internal/api/testutil.ConfigWithDisabledNetwork(...)` now owns the config construction and the three package-local helpers delegate to it. + - Verified: focused `go test ./internal/api/httpapi ./internal/api/udsapi ./internal/api/core ./internal/config ./internal/daemon ./internal/testutil/e2e` passed, then `make verify` passed. diff --git a/.compozy/tasks/network-default/reviews-001/issue_002.md b/.compozy/tasks/network-default/reviews-001/issue_002.md new file mode 100644 index 000000000..24ca01234 --- /dev/null +++ b/.compozy/tasks/network-default/reviews-001/issue_002.md @@ -0,0 +1,96 @@ +--- +status: resolved +file: internal/config/bootstrap_test.go +line: 192 +author: coderabbitai[bot] +provider_ref: thread:PRRT_kwDOR5y4QM58o_sg,comment:PRRC_kwDOR5y4QM66AfBM +--- + +# Issue 002: _⚠️ Potential issue_ | _🟠 Major_ +## Review Comment + +_⚠️ Potential issue_ | _🟠 Major_ + +**Use required `t.Run("Should...")` subtest pattern for these new scenarios** + +These new cases are added as standalone tests; project test guidelines require table-driven subtests with `t.Run("Should...")` for test cases. + +
+♻️ Suggested structure + +```diff +-func TestSaveBootstrapConfigFirstRunKeepsNetworkEnabledByDefault(t *testing.T) { +- t.Parallel() +- ... +-} +- +-func TestSaveBootstrapConfigPreservesExplicitNetworkDisable(t *testing.T) { +- t.Parallel() +- ... +-} ++func TestSaveBootstrapConfigNetworkBehavior(t *testing.T) { ++ t.Parallel() ++ ++ tests := []struct { ++ name string ++ seed string ++ }{ ++ { ++ name: "ShouldKeepNetworkEnabledByDefaultOnFirstRun", ++ seed: "", ++ }, ++ { ++ name: "ShouldPreserveExplicitNetworkDisable", ++ seed: "[network]\nenabled = false\ndefault_channel = \"legacy\"\n", ++ }, ++ } ++ ++ for _, tc := range tests { ++ tc := tc ++ t.Run(tc.name, func(t *testing.T) { ++ t.Parallel() ++ // arrange/act/assert... ++ }) ++ } ++} +``` +
+ + +As per coding guidelines, `**/*_test.go`: “MUST use t.Run("Should...") pattern for ALL test cases” and “Use table-driven tests with subtests (`t.Run`) as default pattern for Go tests”. + +
+🤖 Prompt for AI Agents + +``` +Verify each finding against the current code and only fix it if needed. + +In `@internal/config/bootstrap_test.go` around lines 110 - 192, The two standalone +tests (TestSaveBootstrapConfigFirstRunKeepsNetworkEnabledByDefault and +TestSaveBootstrapConfigPreservesExplicitNetworkDisable) violate the project +guideline requiring table-driven subtests; refactor them into a single +table-driven test that iterates over cases and calls t.Run("Should ...", func(t +*testing.T){ ... }) for each scenario, referencing the same helpers +(ResolveHomePathsFrom, SaveBootstrapConfig, LoadGlobalConfig, writeFile) inside +each subtest; ensure each subtest calls t.Parallel() as appropriate, uses +descriptive "Should..." names, and asserts the same expectations currently in +the two functions (first-run default network enabled and preserving explicit +network disabled + default_channel) so behavior and file contents checks remain +unchanged. +``` + +
+ + + + + +## Triage + +- Decision: `VALID` +- Notes: + - The current file still has two standalone tests, `TestSaveBootstrapConfigFirstRunKeepsNetworkEnabledByDefault` and `TestSaveBootstrapConfigPreservesExplicitNetworkDisable`, rather than a shared table-driven subtest. + - Root cause: the new network-default scenarios were added incrementally as top-level tests instead of following the repo's default `t.Run("Should...")` pattern used elsewhere in `internal/config`. + - Fix approach: collapse the two scenarios into one table-driven test with `Should...` subtest names, keep the existing assertions intact, and preserve per-case parallel execution. + - Implemented: the standalone tests were merged into `TestSaveBootstrapConfigNetworkBehavior` with parallel `Should...` subtests and the original expectations preserved. + - Verified: focused `go test ./internal/api/httpapi ./internal/api/udsapi ./internal/api/core ./internal/config ./internal/daemon ./internal/testutil/e2e` passed, then `make verify` passed. diff --git a/.compozy/tasks/network-default/reviews-001/issue_003.md b/.compozy/tasks/network-default/reviews-001/issue_003.md new file mode 100644 index 000000000..3f0f02573 --- /dev/null +++ b/.compozy/tasks/network-default/reviews-001/issue_003.md @@ -0,0 +1,66 @@ +--- +status: resolved +file: internal/daemon/daemon_test.go +line: 4350 +author: coderabbitai[bot] +provider_ref: thread:PRRT_kwDOR5y4QM58o_sk,comment:PRRC_kwDOR5y4QM66AfBQ +--- + +# Issue 003: _⚠️ Potential issue_ | _🟡 Minor_ +## Review Comment + +_⚠️ Potential issue_ | _🟡 Minor_ + +**Add compile-time interface assertions for new test doubles** + +These newly added methods/types are interface-contract critical for this test path; explicit compile-time checks will prevent accidental drift. + +
+🧩 Suggested addition + +```diff ++var ( ++ _ networkBindableSessionManager = (*fakeSessionManager)(nil) ++ _ syntheticPrompter = (*fakeSessionManager)(nil) ++ _ syntheticPrompter = nonBindableHarnessSessionManager{} ++) +``` +
+ + +As per coding guidelines, `**/*.go`: “Use compile-time interface verification: `var _ Interface = (*Type)(nil)`”. + + +Also applies to: 4476-4491 + +
+🤖 Prompt for AI Agents + +``` +Verify each finding against the current code and only fix it if needed. + +In `@internal/daemon/daemon_test.go` around lines 4333 - 4350, Add compile-time +interface assertions for the test double type to prevent future drift: add lines +like `var _ session.Manager = (*fakeSessionManager)(nil)` (or the concrete +session interface type used in production) next to the fakeSessionManager +definition to assert it implements SetNetworkPeerLifecycle, SetTurnEndNotifier, +PromptNetwork, and IsPrompting; also add the same form of assertion for any +other newly added test doubles mentioned (e.g., the types around lines +4476-4491) so the compiler verifies the interface contract. +``` + +
+ + + + + +## Triage + +- Decision: `VALID` +- Notes: + - The new daemon test doubles currently rely on implicit interface satisfaction only; there are no compile-time assertions covering `fakeSessionManager`, `fakeNetworkBindableSessionManager`, or the synthetic prompt wrapper types near the cited lines. + - Root cause: new test helpers were added for the network-binding path without the repo-standard interface assertions that would catch future drift at compile time. + - Fix approach: add explicit `var _ ...` assertions adjacent to the test double declarations for the production/test interfaces they are expected to satisfy. + - Implemented: compile-time assertions now pin the session-manager, network-bindable, and synthetic-prompter test doubles to their intended interfaces. + - Verified: focused `go test ./internal/api/httpapi ./internal/api/udsapi ./internal/api/core ./internal/config ./internal/daemon ./internal/testutil/e2e` passed, then `make verify` passed. diff --git a/.compozy/tasks/network-default/reviews-001/issue_004.md b/.compozy/tasks/network-default/reviews-001/issue_004.md new file mode 100644 index 000000000..456fa7cb4 --- /dev/null +++ b/.compozy/tasks/network-default/reviews-001/issue_004.md @@ -0,0 +1,28 @@ +--- +status: resolved +file: internal/testutil/e2e/runtime_harness_test.go +line: 35 +severity: nitpick +author: coderabbitai[bot] +provider_ref: review:4151161262,nitpick_hash:c37851268e75 +review_hash: c37851268e75 +source_review_id: "4151161262" +source_review_submitted_at: "2026-04-21T22:49:44Z" +--- + +# Issue 004: Split the three network scenarios into subtests. +## Review Comment + +This packs three distinct behaviors into one test body, so the first failure hides the rest and the cases cannot be parallelized independently. A small table with `t.Run("Should ...")` would fit the suite better. + +As per coding guidelines, `**/*_test.go`: "Use table-driven tests with subtests (`t.Run`) as default pattern for Go tests" and "Add `t.Parallel()` to independent subtests in Go tests". + +## Triage + +- Decision: `VALID` +- Notes: + - `TestPrepareRuntimeLayoutUsesEnabledNetworkByDefaultAndAllowsExplicitDisable` currently checks three distinct behaviors in one body: default enablement, explicit disablement, and `EnableNetwork` overriding a disabled seed. + - Root cause: the test was written as one linear scenario instead of the repo-default subtest structure, which hides later failures and blocks independent parallelization. + - Fix approach: rewrite the test as a small table-driven suite with `Should...` subtests and keep the current network expectations per scenario. + - Implemented: the three scenarios now run as parallel `Should...` subtests with the same enabled/disabled assertions. + - Verified: focused `go test ./internal/api/httpapi ./internal/api/udsapi ./internal/api/core ./internal/config ./internal/daemon ./internal/testutil/e2e` passed, then `make verify` passed. diff --git a/internal/api/core/test_helpers_test.go b/internal/api/core/test_helpers_test.go index 5c7b55c0a..501329914 100644 --- a/internal/api/core/test_helpers_test.go +++ b/internal/api/core/test_helpers_test.go @@ -45,6 +45,10 @@ type handlerFixture struct { HomePaths aghconfig.HomePaths } +func testConfigWithDisabledNetwork(homePaths aghconfig.HomePaths) aghconfig.Config { + return testutil.ConfigWithDisabledNetwork(homePaths) +} + func newHandlerFixture( t *testing.T, manager testutil.StubSessionManager, @@ -121,7 +125,7 @@ func newHandlerFixtureWithAutomationAndTasks( gin.SetMode(gin.TestMode) homePaths := testutil.NewTestHomePaths(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "127.0.0.1" cfg.HTTP.Port = 2123 cfg.Daemon.Socket = "/tmp/api-core-test.sock" diff --git a/internal/api/httpapi/helpers_test.go b/internal/api/httpapi/helpers_test.go index 90e5d0841..385cb8fe5 100644 --- a/internal/api/httpapi/helpers_test.go +++ b/internal/api/httpapi/helpers_test.go @@ -248,7 +248,7 @@ func newTestHandlersWithAutomationBridgesTasksAndWorkspace( ) *Handlers { t.Helper() - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "127.0.0.1" cfg.HTTP.Port = 2123 @@ -293,7 +293,7 @@ func newTestHandlersWithResources( ) *Handlers { t.Helper() - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "127.0.0.1" cfg.HTTP.Port = 2123 @@ -326,7 +326,7 @@ func newTestHandlersWithResourcesAndAuth( t.Helper() homePaths := newTestHomePaths(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "127.0.0.1" cfg.HTTP.Port = 2123 @@ -360,7 +360,7 @@ func newTestHandlersWithSettingsAndExtensions( ) *Handlers { t.Helper() - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = boundHost cfg.HTTP.Port = 2123 @@ -418,6 +418,10 @@ func newTestHomePaths(t *testing.T) aghconfig.HomePaths { return testutil.NewTestHomePaths(t) } +func testConfigWithDisabledNetwork(homePaths aghconfig.HomePaths) aghconfig.Config { + return testutil.ConfigWithDisabledNetwork(homePaths) +} + func writeAgentDef(t *testing.T, homePaths aghconfig.HomePaths, name string) { t.Helper() testutil.WriteAgentDef(t, homePaths, name) diff --git a/internal/api/httpapi/server_test.go b/internal/api/httpapi/server_test.go index 3749e6d52..1864985a0 100644 --- a/internal/api/httpapi/server_test.go +++ b/internal/api/httpapi/server_test.go @@ -33,7 +33,7 @@ func TestNewHonorsOptionsAndDefaults(t *testing.T) { store := memory.NewStore(filepath.Join(t.TempDir(), "memory")) dream := &stubDreamTrigger{} extensionService := &stubExtensionService{} - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "127.0.0.1" cfg.HTTP.Port = freeTCPPort(t) @@ -143,7 +143,7 @@ func TestNewRejectsResourceAuthWithoutResourceService(t *testing.T) { func TestServerStartAndShutdownServeRequests(t *testing.T) { homePaths := newTestHomePaths(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "127.0.0.1" cfg.HTTP.Port = freeTCPPort(t) @@ -222,7 +222,7 @@ func TestServerStartAndShutdownServeRequests(t *testing.T) { func TestServerStartRejectsNilContextAndDuplicateStart(t *testing.T) { homePaths := newTestHomePaths(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "127.0.0.1" cfg.HTTP.Port = freeTCPPort(t) @@ -255,7 +255,7 @@ func TestLoopbackServerAllowsSettingsAndExtensionMutations(t *testing.T) { t.Parallel() homePaths := newTestHomePaths(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "127.0.0.1" cfg.HTTP.Port = freeTCPPort(t) @@ -463,7 +463,7 @@ func TestLoopbackServerRejectsMismatchedSettingsItemNames(t *testing.T) { t.Parallel() homePaths := newTestHomePaths(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "127.0.0.1" cfg.HTTP.Port = freeTCPPort(t) @@ -556,7 +556,7 @@ func TestLoopbackServerMapsDuplicateExtensionInstallToConflict(t *testing.T) { t.Parallel() homePaths := newTestHomePaths(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "127.0.0.1" cfg.HTTP.Port = freeTCPPort(t) @@ -609,7 +609,7 @@ func TestNonLoopbackServerBlocksSettingsAndExtensionMutationsButKeepsReads(t *te t.Parallel() homePaths := newTestHomePaths(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "0.0.0.0" cfg.HTTP.Port = freeTCPPort(t) @@ -816,7 +816,7 @@ func decodeServerJSON(t *testing.T, resp *http.Response, dest any) { func TestServerStartReportsListenFailure(t *testing.T) { homePaths := newTestHomePaths(t) port := freeTCPPort(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.HTTP.Host = "127.0.0.1" cfg.HTTP.Port = port diff --git a/internal/api/testutil/apitest.go b/internal/api/testutil/apitest.go index 29bd0a669..c1a237dba 100644 --- a/internal/api/testutil/apitest.go +++ b/internal/api/testutil/apitest.go @@ -35,6 +35,13 @@ import ( var ErrStubWorkspaceServiceNotImplemented = errors.New("stub workspace service method not implemented") +// ConfigWithDisabledNetwork returns a default test config with networking turned off. +func ConfigWithDisabledNetwork(homePaths aghconfig.HomePaths) aghconfig.Config { + cfg := aghconfig.DefaultWithHome(homePaths) + cfg.Network.Enabled = false + return cfg +} + type StubSessionManager struct { CreateFn func(context.Context, session.CreateOpts) (*session.Session, error) ListFn func() []*session.Info diff --git a/internal/api/udsapi/helpers_test.go b/internal/api/udsapi/helpers_test.go index 08abf5b35..0baab0a2c 100644 --- a/internal/api/udsapi/helpers_test.go +++ b/internal/api/udsapi/helpers_test.go @@ -218,6 +218,7 @@ func newTestHandlersWithSettingsAndExtensions( ) *Handlers { t.Helper() + cfg := testConfigWithDisabledNetwork(homePaths) return newHandlers(&handlerConfig{ sessions: stubSessionManager{}, tasks: stubTaskManager{}, @@ -227,7 +228,7 @@ func newTestHandlersWithSettingsAndExtensions( settingsRestart: restart, extensions: extensions, homePaths: homePaths, - config: aghconfig.DefaultWithHome(homePaths), + config: cfg, logger: discardLogger(), startedAt: time.Date(2026, 4, 3, 12, 0, 0, 0, time.UTC), now: func() time.Time { return time.Date(2026, 4, 3, 12, 0, 1, 0, time.UTC) }, @@ -249,6 +250,7 @@ func newTestHandlersWithRuntime( ) *Handlers { t.Helper() + cfg := testConfigWithDisabledNetwork(homePaths) return newHandlers(&handlerConfig{ sessions: manager, tasks: tasks, @@ -257,7 +259,7 @@ func newTestHandlersWithRuntime( bridges: bridges, workspaces: workspaces, homePaths: homePaths, - config: aghconfig.DefaultWithHome(homePaths), + config: cfg, logger: discardLogger(), startedAt: time.Date(2026, 4, 3, 12, 0, 0, 0, time.UTC), now: func() time.Time { return time.Date(2026, 4, 3, 12, 0, 1, 0, time.UTC) }, @@ -288,6 +290,7 @@ func newTestHandlersWithResources( ) *Handlers { t.Helper() + cfg := testConfigWithDisabledNetwork(homePaths) return newHandlers(&handlerConfig{ sessions: manager, tasks: stubTaskManager{}, @@ -295,7 +298,7 @@ func newTestHandlersWithResources( resources: resources, workspaces: stubWorkspaceService{}, homePaths: homePaths, - config: aghconfig.DefaultWithHome(homePaths), + config: cfg, logger: discardLogger(), startedAt: time.Date(2026, 4, 3, 12, 0, 0, 0, time.UTC), now: func() time.Time { return time.Date(2026, 4, 3, 12, 0, 1, 0, time.UTC) }, @@ -319,6 +322,10 @@ func newTestHomePaths(t *testing.T) aghconfig.HomePaths { return testutil.NewTestHomePaths(t) } +func testConfigWithDisabledNetwork(homePaths aghconfig.HomePaths) aghconfig.Config { + return testutil.ConfigWithDisabledNetwork(homePaths) +} + func shortSocketPath(t *testing.T) string { t.Helper() diff --git a/internal/api/udsapi/server_test.go b/internal/api/udsapi/server_test.go index 6a08a03cd..93f764638 100644 --- a/internal/api/udsapi/server_test.go +++ b/internal/api/udsapi/server_test.go @@ -29,7 +29,7 @@ func TestNewHonorsOptionsAndDefaults(t *testing.T) { dream := &stubDreamTrigger{} bridgeService := &stubBridgeService{} extensionService := &stubExtensionService{} - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.Daemon.Socket = socketPath server, err := New( @@ -176,7 +176,7 @@ func TestNewRequiresSessionManagerTaskServiceObserverAndWorkspaceResolver(t *tes func TestServerStartAndShutdownCreatesAndRemovesSocket(t *testing.T) { homePaths := newTestHomePaths(t) socketPath := shortSocketPath(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.Daemon.Socket = socketPath server, err := New( @@ -242,7 +242,7 @@ func TestServerStartAndShutdownCreatesAndRemovesSocket(t *testing.T) { func TestServerStartRejectsNilContextAndDuplicateStart(t *testing.T) { homePaths := newTestHomePaths(t) socketPath := shortSocketPath(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.Daemon.Socket = socketPath server, err := New( @@ -273,7 +273,7 @@ func TestServerStartRejectsNilContextAndDuplicateStart(t *testing.T) { func TestServerStartRejectsRegularFileAtSocketPath(t *testing.T) { homePaths := newTestHomePaths(t) socketPath := shortSocketPath(t) - cfg := aghconfig.DefaultWithHome(homePaths) + cfg := testConfigWithDisabledNetwork(homePaths) cfg.Daemon.Socket = socketPath if err := os.WriteFile(socketPath, []byte("not-a-socket"), 0o600); err != nil { t.Fatalf("os.WriteFile(socketPath) error = %v", err) diff --git a/internal/config/bootstrap_test.go b/internal/config/bootstrap_test.go index 4cba17078..abafddfe0 100644 --- a/internal/config/bootstrap_test.go +++ b/internal/config/bootstrap_test.go @@ -57,6 +57,9 @@ api_key_env = "ANTHROPIC_KEY" if cfg.Memory.Dream.Agent != DefaultAgentName { t.Fatalf("SaveBootstrapConfig() Memory.Dream.Agent = %q, want %q", cfg.Memory.Dream.Agent, DefaultAgentName) } + if !cfg.Network.Enabled { + t.Fatal("SaveBootstrapConfig() Network.Enabled = false, want inherited enabled default") + } reloaded, err := LoadGlobalConfig(homePaths) if err != nil { @@ -79,6 +82,9 @@ api_key_env = "ANTHROPIC_KEY" "claude-sonnet-4-20250514", ) } + if !reloaded.Network.Enabled { + t.Fatal("LoadGlobalConfig() Network.Enabled = false, want inherited enabled default") + } contents, err := os.ReadFile(homePaths.ConfigFile) if err != nil { @@ -101,6 +107,95 @@ api_key_env = "ANTHROPIC_KEY" } } +func TestSaveBootstrapConfigNetworkBehavior(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + seed string + wantEnabled bool + wantDefaultChannel string + wantNetworkSection bool + }{ + { + name: "ShouldKeepNetworkEnabledByDefaultOnFirstRun", + wantDefaultChannel: "default", + wantEnabled: true, + wantNetworkSection: false, + }, + { + name: "ShouldPreserveExplicitNetworkDisable", + seed: ` +[network] +enabled = false +default_channel = "legacy" +`, + wantEnabled: false, + wantDefaultChannel: "legacy", + wantNetworkSection: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + homePaths, err := ResolveHomePathsFrom(filepath.Join(t.TempDir(), "home")) + if err != nil { + t.Fatalf("ResolveHomePathsFrom() error = %v", err) + } + + if strings.TrimSpace(tt.seed) != "" { + writeFile(t, homePaths.ConfigFile, tt.seed) + } + + cfg, err := SaveBootstrapConfig(homePaths, "claude", "claude-sonnet-4-20250514") + if err != nil { + t.Fatalf("SaveBootstrapConfig() error = %v", err) + } + if got := cfg.Network.Enabled; got != tt.wantEnabled { + t.Fatalf("SaveBootstrapConfig() Network.Enabled = %t, want %t", got, tt.wantEnabled) + } + if got := cfg.Network.DefaultChannel; got != tt.wantDefaultChannel { + t.Fatalf("SaveBootstrapConfig() Network.DefaultChannel = %q, want %q", got, tt.wantDefaultChannel) + } + + reloaded, err := LoadGlobalConfig(homePaths) + if err != nil { + t.Fatalf("LoadGlobalConfig() error = %v", err) + } + if got := reloaded.Network.Enabled; got != tt.wantEnabled { + t.Fatalf("LoadGlobalConfig() Network.Enabled = %t, want %t", got, tt.wantEnabled) + } + if got := reloaded.Network.DefaultChannel; got != tt.wantDefaultChannel { + t.Fatalf("LoadGlobalConfig() Network.DefaultChannel = %q, want %q", got, tt.wantDefaultChannel) + } + + contents, err := os.ReadFile(homePaths.ConfigFile) + if err != nil { + t.Fatalf("ReadFile(config) error = %v", err) + } + text := string(contents) + if !tt.wantNetworkSection { + if strings.Contains(text, "[network]") { + t.Fatalf("bootstrap config wrote an unexpected network section:\n%s", text) + } + return + } + + for _, want := range []string{ + "[network]", + `enabled = false`, + `default_channel = "legacy"`, + } { + if !strings.Contains(text, want) { + t.Fatalf("config contents missing %q\n%s", want, text) + } + } + }) + } +} + func TestEnsureBootstrapAgentCreatesAndPreservesManagedAgent(t *testing.T) { t.Parallel() diff --git a/internal/config/config.go b/internal/config/config.go index 2464eb5f4..d77022ae2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -429,7 +429,7 @@ func DefaultWithHome(homePaths HomePaths) Config { DefaultFireLimit: automationpkg.DefaultFireLimitConfig(), }, Network: NetworkConfig{ - Enabled: false, + Enabled: true, DefaultChannel: "default", Port: -1, MaxPayload: 1 << 20, diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d56df34d3..8eb52e604 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1482,6 +1482,12 @@ func TestLoadMissingConfigReturnsDefaults(t *testing.T) { !slices.Equal(cfg.Skills.DisabledSkills, want.Skills.DisabledSkills) { t.Fatalf("Load() Skills = %#v, want %#v", cfg.Skills, want.Skills) } + if cfg.Network != want.Network { + t.Fatalf("Load() Network = %#v, want %#v", cfg.Network, want.Network) + } + if !cfg.Network.Enabled { + t.Fatal("Load() Network.Enabled = false, want true by default") + } } func TestDefaultConfigUsesResolvedHomePaths(t *testing.T) { @@ -1506,6 +1512,9 @@ func TestDefaultConfigUsesResolvedHomePaths(t *testing.T) { if got, want := cfg.Skills.PollInterval, 3*time.Second; got != want { t.Fatalf("defaultConfig() Skills.PollInterval = %s, want %s", got, want) } + if !cfg.Network.Enabled { + t.Fatal("defaultConfig() Network.Enabled = false, want true") + } if got, want := cfg.Network.DefaultChannel, "default"; got != want { t.Fatalf("defaultConfig() Network.DefaultChannel = %q, want %q", got, want) } @@ -1517,6 +1526,37 @@ func TestDefaultConfigUsesResolvedHomePaths(t *testing.T) { } } +func TestLoadRespectsExplicitNetworkDisable(t *testing.T) { + workspaceRoot := t.TempDir() + homeRoot := filepath.Join(t.TempDir(), "home") + t.Setenv("AGH_HOME", homeRoot) + + homePaths, err := ResolveHomePaths() + if err != nil { + t.Fatalf("ResolveHomePaths() error = %v", err) + } + if err := EnsureHomeLayout(homePaths); err != nil { + t.Fatalf("EnsureHomeLayout() error = %v", err) + } + + writeFile(t, homePaths.ConfigFile, ` +[network] +enabled = false +default_channel = "operators" +`) + + cfg, err := Load(WithWorkspaceRoot(workspaceRoot)) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if cfg.Network.Enabled { + t.Fatal("Load() Network.Enabled = true, want explicit false override to win") + } + if got, want := cfg.Network.DefaultChannel, "operators"; got != want { + t.Fatalf("Load() Network.DefaultChannel = %q, want %q", got, want) + } +} + func TestNetworkConfigValidateRejectsInvalidValues(t *testing.T) { t.Parallel() diff --git a/internal/daemon/daemon_test.go b/internal/daemon/daemon_test.go index 106071b83..27a7c9e75 100644 --- a/internal/daemon/daemon_test.go +++ b/internal/daemon/daemon_test.go @@ -345,7 +345,9 @@ func TestShutdownClosesResourceReconcileDriver(t *testing.T) { func TestBootEnabledNetworkLateBindsSessionCallbacksAndPersistsSafeDiagnostics(t *testing.T) { homePaths := testHomePaths(t) cfg := testConfig(t, homePaths) - cfg.Network.Enabled = true + if !cfg.Network.Enabled { + t.Fatal("testConfig() Network.Enabled = false, want true by default") + } bindableSessions := newFakeNetworkBindableSessionManager() d := newTestDaemon(t, homePaths, &cfg) @@ -420,7 +422,11 @@ func TestBootEnabledNetworkRejectsSessionManagersMissingBindingSurface(t *testin return &recordingRegistry{path: homePaths.DatabaseFile}, nil } d.newSessionManager = func(context.Context, SessionManagerDeps) (SessionManager, error) { - return &fakeSessionManager{}, nil + base := &fakeSessionManager{} + return nonBindableHarnessSessionManager{ + SessionManager: base, + syntheticPrompter: base, + }, nil } d.newObserver = func(context.Context, RuntimeDeps) (Observer, error) { return &fakeObserver{}, nil @@ -4324,6 +4330,25 @@ func (f *fakeSessionManager) ApprovePermission(context.Context, string, acp.Appr return nil } +func (f *fakeSessionManager) SetNetworkPeerLifecycle(session.NetworkPeerLifecycle) {} + +func (f *fakeSessionManager) SetTurnEndNotifier(session.TurnEndNotifier) {} + +func (f *fakeSessionManager) PromptNetwork( + context.Context, + string, + string, + ...acp.PromptNetworkMeta, +) (<-chan acp.AgentEvent, error) { + ch := make(chan acp.AgentEvent) + close(ch) + return ch, nil +} + +func (f *fakeSessionManager) IsPrompting(string) bool { + return false +} + func (f *fakeSessionManager) recordSyntheticEvent( sessionID string, info *session.Info, @@ -4448,6 +4473,32 @@ func (f *fakeNetworkBindableSessionManager) IsPrompting(sessionID string) bool { return f.prompting[sessionID] } +type syntheticPrompter interface { + PromptSynthetic(context.Context, string, session.SyntheticPromptOpts) (<-chan acp.AgentEvent, error) +} + +type nonBindableHarnessSessionManager struct { + SessionManager + syntheticPrompter syntheticPrompter +} + +var ( + _ SessionManager = (*fakeSessionManager)(nil) + _ SessionManager = (*fakeNetworkBindableSessionManager)(nil) + _ SessionManager = nonBindableHarnessSessionManager{} + _ networkBindableSessionManager = (*fakeNetworkBindableSessionManager)(nil) + _ syntheticPrompter = (*fakeSessionManager)(nil) + _ syntheticPrompter = nonBindableHarnessSessionManager{} +) + +func (m nonBindableHarnessSessionManager) PromptSynthetic( + ctx context.Context, + id string, + opts session.SyntheticPromptOpts, +) (<-chan acp.AgentEvent, error) { + return m.syntheticPrompter.PromptSynthetic(ctx, id, opts) +} + type fakeNetworkRuntime struct { mu sync.Mutex status *network.Status diff --git a/internal/testutil/e2e/runtime_harness_test.go b/internal/testutil/e2e/runtime_harness_test.go index ce06c74cb..aa4a2dd70 100644 --- a/internal/testutil/e2e/runtime_harness_test.go +++ b/internal/testutil/e2e/runtime_harness_test.go @@ -5,6 +5,8 @@ import ( "os" "path/filepath" "testing" + + aghconfig "github.com/pedronauck/agh/internal/config" ) func TestPrepareRuntimeLayoutCreatesIsolatedPaths(t *testing.T) { @@ -30,17 +32,53 @@ func TestPrepareRuntimeLayoutCreatesIsolatedPaths(t *testing.T) { } } -func TestPrepareRuntimeLayoutEnablesNetworkOnlyWhenRequested(t *testing.T) { +func TestPrepareRuntimeLayoutUsesEnabledNetworkByDefaultAndAllowsExplicitDisable(t *testing.T) { t.Parallel() - disabled := prepareRuntimeLayout(t, RuntimeHarnessOptions{}) - if disabled.Config.Network.Enabled { - t.Fatal("disabled.Config.Network.Enabled = true, want false by default") + tests := []struct { + name string + opts RuntimeHarnessOptions + want bool + }{ + { + name: "ShouldEnableNetworkByDefault", + opts: RuntimeHarnessOptions{}, + want: true, + }, + { + name: "ShouldAllowExplicitDisableFromConfigSeed", + opts: RuntimeHarnessOptions{ + ConfigSeed: ConfigSeedOptions{ + Mutate: func(cfg *aghconfig.Config) { + cfg.Network.Enabled = false + }, + }, + }, + want: false, + }, + { + name: "ShouldOverrideDisabledSeedWhenEnableNetworkIsRequested", + opts: RuntimeHarnessOptions{ + EnableNetwork: true, + ConfigSeed: ConfigSeedOptions{ + Mutate: func(cfg *aghconfig.Config) { + cfg.Network.Enabled = false + }, + }, + }, + want: true, + }, } - enabled := prepareRuntimeLayout(t, RuntimeHarnessOptions{EnableNetwork: true}) - if !enabled.Config.Network.Enabled { - t.Fatal("enabled.Config.Network.Enabled = false, want true when EnableNetwork is requested") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + layout := prepareRuntimeLayout(t, tt.opts) + if got := layout.Config.Network.Enabled; got != tt.want { + t.Fatalf("layout.Config.Network.Enabled = %t, want %t", got, tt.want) + } + }) } } diff --git a/packages/site/content/runtime/core/configuration/config-toml.mdx b/packages/site/content/runtime/core/configuration/config-toml.mdx index 4991ad23f..020ed503e 100644 --- a/packages/site/content/runtime/core/configuration/config-toml.mdx +++ b/packages/site/content/runtime/core/configuration/config-toml.mdx @@ -47,7 +47,7 @@ will fail config loading as an unknown key. | `[[automation.jobs]]` | Scheduled automation jobs. | empty list | | `[[automation.triggers]]` | Event-driven automation triggers. | empty list | | `[[hooks.declarations]]` | Config-defined runtime hooks. | empty list | -| `[network]` | Experimental AGH network runtime. | disabled, channel `default` | +| `[network]` | Experimental AGH network runtime. | enabled, channel `default` | ## Load And Merge Order @@ -209,7 +209,7 @@ env = { HOOK_LOG = "1" } agent_name = "general" [network] -enabled = false +enabled = true default_channel = "default" port = -1 max_payload = 1048576 @@ -482,7 +482,7 @@ context.post_compact | Field | Type | Default | Valid values | Description | | ----------------- | --------------- | --------- | ---------------------------------------------- | ---------------------------------------------------- | -| `enabled` | boolean | `false` | `true` or `false` | Enables the embedded AGH network runtime. | +| `enabled` | boolean | `true` | `true` or `false` | Enables the embedded AGH network runtime. | | `default_channel` | string | `default` | Regex `^[a-z0-9][a-z0-9_-]{0,63}$` | Default network channel for sessions. | | `port` | integer | `-1` | `-1` or `1` through `65535` | Network listener port. `-1` lets the runtime choose. | | `max_payload` | integer bytes | `1048576` | Positive integer no greater than `2147483647`. | Maximum network payload size. | diff --git a/web/src/storybook/web-storybook-stories-and-fixtures.test.tsx b/web/src/storybook/web-storybook-stories-and-fixtures.test.tsx index 4a796561e..2fdf29ad1 100644 --- a/web/src/storybook/web-storybook-stories-and-fixtures.test.tsx +++ b/web/src/storybook/web-storybook-stories-and-fixtures.test.tsx @@ -12,23 +12,29 @@ import { workspaceDetailFixture, workspaceFixtures } from "@/systems/workspace/m describe("storybook story and fixture regressions", () => { const fromWeb = (path: string) => resolve(process.cwd(), path); - it("loads the edited story modules", async () => { - const modules = await Promise.all([ - import("@/systems/knowledge/components/stories/knowledge-detail-panel.stories"), - import("@/systems/knowledge/components/stories/knowledge-list-panel.stories"), - import("@/systems/network/components/stories/network-channels-list-panel.stories"), - import("@/systems/automation/components/stories/automation-editor-dialog.stories"), - import("@/systems/session/components/stories/copy-button.stories"), - import("@/systems/session/components/tool-renderers/stories/read-content.stories"), - import("@/systems/session/components/tool-renderers/stories/search-content.stories"), - ]); + it( + "loads the edited story modules", + { + timeout: 20_000, + }, + async () => { + const modules = await Promise.all([ + import("@/systems/knowledge/components/stories/knowledge-detail-panel.stories"), + import("@/systems/knowledge/components/stories/knowledge-list-panel.stories"), + import("@/systems/network/components/stories/network-channels-list-panel.stories"), + import("@/systems/automation/components/stories/automation-editor-dialog.stories"), + import("@/systems/session/components/stories/copy-button.stories"), + import("@/systems/session/components/tool-renderers/stories/read-content.stories"), + import("@/systems/session/components/tool-renderers/stories/search-content.stories"), + ]); - expect(modules).toHaveLength(7); + expect(modules).toHaveLength(7); - for (const module of modules) { - expect(module.default).toBeDefined(); + for (const module of modules) { + expect(module.default).toBeDefined(); + } } - }); + ); it("keeps the scoped story and fixture source aligned with the review fixes", async () => { const sources = await Promise.all([ diff --git a/web/vitest.config.ts b/web/vitest.config.ts index d8fe76812..2cf67fb87 100644 --- a/web/vitest.config.ts +++ b/web/vitest.config.ts @@ -14,6 +14,7 @@ export default defineConfig({ name: "web", environment: "jsdom", globals: true, + testTimeout: 10_000, include: ["src/**/*.{test,spec}.{ts,tsx}", "e2e/**/*.test.ts", "tests/**/*.test.{ts,tsx}"], exclude: ["**/node_modules/**", "**/dist/**", "tests/visual/*.spec.ts"], setupFiles: ["./src/test-setup.ts"],