[ericcurtin]

Summary

Fall back to the system active_gateway when the user one is missing or invalid, like the CLI.

Related Issue

Closes #4517

Changes

• resolveActiveGateway tries the user file, then <system dir>/active_gateway.
• Share one file reader for both lookups.
• ListGateways now marks the system default as Active.
• Add a Go SDK docs note on active-gateway discovery.
• Add fallback tests; isolate no-active-gateway tests from /etc/openshell.

Testing

• Checks appropriate to the affected code and behavior pass
• Unit tests added/updated (if applicable)
• E2E tests added/updated (if applicable)

Ran go test ./... and golangci-lint run in sdk/go.

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs updated (if applicable)

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[ericcurtin]

@TaylorMutch @SDAChess could you please triage #4517 so the issue check can pass? PTAL, and /ok to test 3ded94f78ef8b2f216f56dcfdc962ba655e526aa if it looks good. Thank you!

[ericcurtin]

@krishicks @mrunalp could you please accept #4517 so the issue check can pass? PTAL, and /ok to test 3ded94f78ef8b2f216f56dcfdc962ba655e526aa if it looks good. Thank you!

[rhuss]

Heads-up: the Go SDK moves to ai-openshell/sdk-go on 2026-11-03 (details in #5031). For this PR that means:

• merged in core by the end of that day: it moves with the history, nothing to do
• still open: it is ported to the new repository with your commits and authorship kept (git format-patch and git am), and a comment here links the replacement before this PR is closed

Nothing to do on your side either way.

cc @TaylorMutch, a review by Monday 2026-11-02 would let this land in core.

[rhuss]

The Go SDK freeze is tomorrow, Tuesday 2026-11-03, at the end of the day (#5031). This fix is small and SDK-only, so it would be good to land it in core before then. Two things block it:

• bug(sdk/go): no system-level active gateway fallback; OPENSHELL_SYSTEM_GATEWAY_DIR ignored #4517 needs state:accepted for the issue check
• CI hasn't run yet (/ok to test), and there's no review

@TaylorMutch @pimlock, could one of you accept the issue and review? If it doesn't make it, it gets ported to ai-openshell/sdk-go with the original commits, as announced above.

[purp]

/ok to test 3ded94f

[rhuss]

cc-review Summary

Thanks for this fix, Eric. The change is small, follows the CLI's load_active_gateway closely, and the new tests are hermetic against a real /etc/openshell on the host.

This is an agent-assisted review: several reviewer agents read the diff and I curated their findings. Any scenarios below were derived from reading the code and were not executed. Everything here is a non-blocking suggestion (Minor or Notable), so none of it needs to hold the PR. Once you have had a look, this seems ready for a maintainer.

What Went Well

• paths.go readActiveGatewayFile (lines 161-178): one shared reader applies the same trim, empty check and validateGatewayName to both files, so a value like ../escape in the system file cannot traverse out of the gateways directory.
• paths.go resolveActiveGateway (lines 144-159): the user-then-system order mirrors load_active_gateway in crates/openshell-bootstrap/src/metadata.rs, including the invalid-user fallback asked for in the triage of #4517.
• paths.go line 155: reusing systemConfigDir() means the OPENSHELL_SYSTEM_GATEWAY_DIR override and its empty/relative rejection apply to the active gateway lookup exactly as they do to gateway directories.
• paths_test.go and gateway_test.go: the no-active-gateway tests now pin OPENSHELL_SYSTEM_GATEWAY_DIR to a temp dir, and TestResolveActiveGateway_PrefersUser writes different names to both files, so it checks precedence rather than mere presence.
• gateway_test.go TestLoadConfig_SystemActiveGateway (line 398): an end-to-end check of the reproduction from #4517 through the public LoadConfig, asserting both Name and Source == SourceSystem.

Findings

Severity	File	Description	Source
Minor	sdk/go/openshell/v1/gateway/paths.go:150	A CLI-valid but Go-invalid user name silently falls back to the system gateway	gator
Minor	sdk/go/openshell/v1/gateway/paths.go:155	A rejected system active_gateway is not mentioned in the returned error	production
Minor	sdk/go/openshell/v1/gateway/paths.go:146	No test for the userConfigDir() error branch falling back to the system file	test-quality
Minor	sdk/go/openshell/v1/gateway/gateway_test.go:398	No test for ListGateways marking a system-default gateway as Active	test-quality
Minor	sdk/go/openshell/v1/gateway/paths_test.go:265	InvalidSystemIsIgnored does not check that the error describes the user file	test-quality
Notable	sdk/go/openshell/v1/gateway/gateway.go:120	ListGateways now reports a system-default gateway as Active (summary only)	goal-alignment

The comments on paths.go:155 and paths_test.go:265 pull in opposite directions (surface the system error, or pin the user-only error). Either choice is fine; the point is to pick one and pin it with a test.

Not posted inline (the location, gateway.go line 120, is outside the PR diff):

• Notable: ListGateways calls resolveActiveGateway, so with only a system active_gateway it now returns that gateway with Active: true (before this PR it was false). This matches the CLI's gateway list and the existing Info.Active doc ("whether this is the currently active gateway"), so the behavior itself looks right. A one-line mention in the PR description (and optionally in the ListGateways doc comment) would help consumers who read Active as "explicitly selected by the user". The matching test gap is in the gateway_test.go:398 comment. Source: goal-alignment agent

Review Details

• Findings posted: 6 (5 inline, 1 in this summary)
• Findings reviewed and not posted: 0
• Gate outcome: PASS (no Critical or Important findings)
• Participating reviewers: gator, production, test-quality, goal-alignment (architecture, correctness and security also ran and reported no findings)

[purp]

gator-agent

PR Review Status

The independent code review found no blocking defects. The Go SDK now tries the user active gateway before the system default, validates both names through the same reader, and tests fallback, user precedence, invalid entries, and system-backed LoadConfig.

@ericcurtin, I checked your request to accept #4517 and start CI: the issue now has state:accepted, and Branch Checks (including Go SDK) passed after @purp authorized this head. The issue-acceptance failure is from before acceptance; its logs explicitly request a rerun after acceptance.

Action required: a maintainer must approve the Trivy Changes run, which is awaiting workflow approval, and rerun PR Issue Acceptance. For the changed SDK gateway-selection behavior, please add a short explanation to docs/sdk/go.mdx, or have a maintainer explain here why the existing published gateway docs suffice.

Blocking code findings: None.

Carried findings: None.

Gator metadata

• Validation: Focused Go SDK bug fix for accepted issue bug(sdk/go): no system-level active gateway fallback; OPENSHELL_SYSTEM_GATEWAY_DIR ignored #4517.
• Docs: Go API comments updated; no docs/ change or maintainer-authored exemption for the direct SDK behavior change. Existing gateway docs describe the system layout, but the Go SDK page does not explain active-gateway discovery.
• Checks: Branch Checks and Helm Lint gates passed; DCO and vouch passed. PR Issue Acceptance has a stale failure. Required Trivy Changes has not executed and is awaiting approval. Maintainer approval is also absent.
• E2E: No test labels required for this SDK configuration-file lookup change; no sandbox lifecycle, runtime, GPU, Kubernetes, or Windows-specific changes.
• Head SHA: 3ded94f78ef8b2f216f56dcfdc962ba655e526aa
• Base SHA: c4688aa8715ea75d6c39fe468361dbbc16c691fa
• Merge base SHA: c4688aa8715ea75d6c39fe468361dbbc16c691fa
• Patch ID: 533dbaabce3ff9953c6e7532e44d23d4e1c27e49
• Gator payload: 13
• Review mode: initial
• Previous reviewed SHA: none
• Review budget exhausted: no
• Maintainer decision required: no convergence decision needed
• Next state: gator:blocked
• Blocked reason: test_dispatch_required

[ericcurtin]

Pushed 017959a: pinned the user-file error contract, added the three requested tests (shared helper), noted ListGateways system-default Active in docs and doc comment, and added a Go SDK docs section. CLI-valid but Go-invalid user names are left as a follow-up.

[rhuss]

Follow-up on my review: I checked the current head (017959a) against my five threads, and a fresh review of the full change found nothing new.

• Fixed: tests for the userConfigDir() error branch, the pinned user-file error contract (doc comment plus TestResolveActiveGateway_InvalidSystemIsIgnored), TestListGateways_SystemActiveStatus, and the error-source assertions with distinct invalid names.
• Deferred, nonblocking: a user active_gateway that the CLI accepts but validateGatewayName rejects (for example my gw) still falls back to the system default without a signal. A tracking issue for that follow-up would help.

Ready for a maintainer review.

[ericcurtin]

@TaylorMutch @purp Addressed the review comments in 017959a, and the Go SDK freeze is today. PTAL, and /ok to test 017959a020502452f4a36d6bb13119b63f9c272a if it looks good. Thank you!

[TaylorMutch]

/ok to test 017959a

[TaylorMutch]

Reviewed active gateway fallback against the CLI and checked user precedence, validation, error handling, and listing. No blocking findings. The gateway package passed three times with the race detector. Required Branch Checks are still pending.