Add package to exit program with relevant exit code.

[ggreer]

This adds Exit() and LogExit(), which detect if an error is a GRPC error, context cancelled, or context deadline exceeded, and exit with the appropriate exit code.

This lets us add tests to make sure that a connector exits with the correct GRPC status if it hits an auth error.

Getting this behavior requires a small change to each connector. Without it, syncs still exit with status code 1 no matter what. The print/exit behavior in main.go should be change from this:

func main() {
	ctx := context.Background()

	_, cmd, err := configschema.DefineConfiguration(ctx, "baton-demo", getConnector, config.Config)
	if err != nil {
		fmt.Fprintln(os.Stderr, err.Error())
		os.Exit(1)
	}

	cmd.Version = version

	err = cmd.Execute()
	if err != nil {
		fmt.Fprintln(os.Stderr, err.Error())
		os.Exit(1)
	}
}

to this:

func main() {
	ctx := context.Background()

	_, cmd, err := configschema.DefineConfiguration(ctx, "baton-demo", getConnector, config.Config)
	if err != nil {
		exit.LogExit(err)
	}

	cmd.Version = version

	err = cmd.Execute()
	if err != nil {
		exit.LogExit(err)
	}
}

[github-actions]

🟡 Suggestion (confidence: high): This silently changes the exit code of every connector's ordinary failure path from 1 to 2, since config.RunConnector is the shared entrypoint for the whole fleet. Non-zero-ness is preserved so != 0 checks are unaffected, but anything that distinguishes specific codes (CI scripts, container orchestration, the platform task runner) will see a different value with no migration note and no pkg/sdk/version.go signal.

Separately, codes.Unknown == 2 is also the exit code the Go runtime uses for an unrecovered panic, and the conventional "usage error" code for CLIs. That makes 2 ambiguous exactly where this package is meant to be the oracle: a test asserting "exited 2 → generic error" also passes when the connector panicked.

[github-actions]

🟡 Suggestion (confidence: medium): exitCode is unexported, so the only public way to learn the code for an error is to terminate the process. The PR's stated goal is asserting that connectors exit with the right gRPC status; exporting this as Code(err error) int would let downstream connectors and their tests map an error to a code without a subprocess harness, and keeps Exit/LogExit as thin wrappers.

[github-actions]

General PR Review: Add package to exit program with relevant exit code.

Blocking Issues: 0 | Suggestions: 3 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 5ad9602efa8e.
Review mode: full
View review run

Review Summary

Scanned the full PR diff (4 files, +145/-8) for security and correctness: the new pkg/exit package plus its two call sites in cmd/baton/main.go and pkg/config/config.go. exitCode is logically correct — I verified the vendored status.FromError contract and confirmed the ok=false fallthrough, the nil-GRPCStatus() case, and the join/wrap orderings all match the table test, and that the only (nil, true) return is the already-handled err == nil case. No security issues and no confident correctness bugs. The three suggestions are about the compatibility surface of the exit-code change rather than defects in the mapping itself.

Risk triage (per docs/BUG_CATCHING.md §2) — MEDIUM, no escalation requested:

• Silence: weak yes — a shifted exit code emits no error, just a different integer.
• Durability: no — nothing is persisted; no c1z, proto, session, or pagination state is touched.
• Uncontrolled dimensions: no — exitCode is a pure function of the error value, with no schedule, scale, or version-pair dependence.
• Consumer distance: yes — exit codes are read by CI, container orchestration, and the platform task runner outside this repo.

Consequence lands at remediation rung ~2 rather than rung 4, because every error path still exits non-zero (minimum code is Canceled = 1), so != 0 checks across the fleet are unaffected; only consumers that discriminate among non-zero codes change behavior. The table test plus the all-codes sweep are the right instruments for this risk class and are already present. No dependency changes: go.mod, go.sum, and vendor/ are untouched, and google.golang.org/grpc is already a direct dependency.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• pkg/exit/exit.go:52 — Generic errors now exit 2 instead of 1 for every connector via config.RunConnector; no migration note and no pkg/sdk/version.go bump (still v0.24.6).
• pkg/exit/exit.go:52 — codes.Unknown (2) collides with Go's unrecovered-panic exit code and the conventional CLI usage-error code, making 2 an ambiguous oracle.
• pkg/exit/exit.go:30 — exitCode is unexported, so callers cannot map an error to a code without terminating the process; consider exporting Code(err error) int.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/exit/exit.go`:
- Around line 30-53: `exitCode` is unexported, so the only public way to learn an
  error's exit code is to call Exit/LogExit and terminate the process. Export it as
  `Code(err error) int` with a doc comment describing the mapping, and reduce
  `Exit` to `os.Exit(Code(err))`. Keep the unexported name as an alias only if
  something depends on it. This lets downstream connectors assert the code in unit
  tests instead of needing a subprocess harness, which is the stated goal of the PR.
- Around line 51-52: the fallback returns `int(codes.Unknown)` == 2. Two problems
  worth deciding on explicitly rather than leaving implicit. First, 2 is also the
  exit code the Go runtime uses for an unrecovered panic, and the conventional
  "usage error" code for CLIs, so a test asserting "exit 2 means generic connector
  error" will also pass when the connector panicked. Consider either offsetting all
  gRPC-derived codes into a non-colliding range (for example `64 + int(code)`), or
  documenting the collision in the package doc comment and in whatever test helper
  asserts on these codes so callers do not treat 2 as unambiguous.

In `pkg/config/config.go`:
- Around line 62-73: `RunConnector` is the shared entrypoint for the whole connector
  fleet, so routing its failures through `exit.LogExit` changes the observable exit
  code of every connector binary: ordinary non-gRPC errors move from 1 to 2, and
  gRPC-derived errors move from 1 to their status code (for example 16 for
  Unauthenticated, 14 for Unavailable). Every error path still exits non-zero, so
  `!= 0` checks are safe, but consumers that discriminate among non-zero codes will
  behave differently. Per the repo review criteria, add a migration/rollout note to
  the PR description spelling out the old-to-new code mapping, and bump
  `pkg/sdk/version.go` (currently `v0.24.6`) with at least a 0.x minor bump so
  downstream repos get a compatibility signal rather than a patch-looking release.

[github-actions]

No blocking issues found.