refactor: enable AGH network by default for new installs

.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

.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.

.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.
+
+<details>
+<summary>♻️ Suggested structure</summary>
+
+```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...
++		})
++	}
++}
+```
+</details>
+
+
+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”.
+
+<details>
+<summary>🤖 Prompt for AI Agents</summary>
+
+```
+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.
+```
+
+</details>
+
+<!-- fingerprinting:phantom:poseidon:hawk -->
+
+<!-- This is an auto-generated comment by CodeRabbit -->
+
+## 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.

.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.
+
+<details>
+<summary>🧩 Suggested addition</summary>
+
+```diff
++var (
++	_ networkBindableSessionManager = (*fakeSessionManager)(nil)
++	_ syntheticPrompter             = (*fakeSessionManager)(nil)
++	_ syntheticPrompter             = nonBindableHarnessSessionManager{}
++)
+```
+</details>
+
+
+As per coding guidelines, `**/*.go`: “Use compile-time interface verification: `var _ Interface = (*Type)(nil)`”.
+
+
+Also applies to: 4476-4491
+
+<details>
+<summary>🤖 Prompt for AI Agents</summary>
+
+```
+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.
+```
+
+</details>
+
+<!-- fingerprinting:phantom:poseidon:hawk -->
+
+<!-- This is an auto-generated comment by CodeRabbit -->
+
+## 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.

.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.

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"

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)

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
 

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