refactor: enforce CLI server import boundary

[Nek-12]

Summary

• move presentation-safe auth, model, session, and LLM error contracts into shared packages
• route remaining local embedded/server wiring through documented CLI serverbridge packages
• add production import-boundary guard with exact bridge allowlist and DTO parity tests

Closes #135

Verification

• ./scripts/test.sh ./shared/architecture ./shared/... ./server/auth ./server/llm ./server/session ./cli/app/... ./cli/builder
• ./scripts/build.sh --output ./bin/builder
• git diff --check

Summary by CodeRabbit

• Refactor
  • Internal architectural restructuring to improve code organization and maintainability. Shared data types and contracts have been consolidated, and CLI composition bridges established to enforce clearer separation between components. No user-facing functionality changes.

[coderabbitai]

Warning

Rate limit exceeded

@Nek-12 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 54 minutes and 13 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d40e3f22-6a20-4dd6-aa35-a936fd6a1f88

📥 Commits

Reviewing files that changed from the base of the PR and between dd2ea58 and 098383f.

📒 Files selected for processing (2)

• cli/app/internal/serverbridge/serverbridge.go
• cli/builder/internal/serverbridge/serverbridge.go

📝 Walkthrough

Walkthrough

This PR systematically eliminates CLI→Server direct imports by extracting shared contract types into new shared/* packages, refactoring server/* to re-export those contracts, and introducing serverbridge adapter packages that CLI code imports instead of server. Multiple CLI files update their imports to route through the new bridge layer.

Changes

CLI Server Decoupling via Shared Contracts and Composition Bridges

Layer / File(s)	Summary
Shared contract foundation shared/auth/types.go, shared/llmerrors/errors.go, shared/modelcontract/types.go, shared/sessioncontract/errors.go	New packages define core authentication state/methods, LLM error classification, model metadata shapes, and session error sentinels that both CLI and server can consume without circular dependencies.
Server package refactoring to re-export shared contracts server/auth/types.go, server/auth/openai_oauth.go, server/auth/gate.go, server/llm/errors.go, server/llm/types.go, server/llm/model_catalog.go, server/session/store.go	Auth type definitions, error sentinels, and startup-gating logic migrate from local implementations to type aliases and function delegations to shared/* packages, consolidating single sources of truth and allowing CLI code to consume shared types directly.
Serverbridge composition/delegation layer cli/app/internal/serverbridge/serverbridge.go, cli/builder/internal/serverbridge/serverbridge.go, cli/app/internal/serverbridge/contracts_test.go	New serverbridge packages aggregate type aliases and wrapper functions that re-export and delegate to underlying auth/OAuth/runtime/server packages, providing a single controlled import entry point for CLI code. Type identity and contract consistency are validated by tests.
CLI migration to serverbridge and shared imports cli/app/internal/authcommand/slash.go, cli/app/internal/authflowadapter/flow.go, cli/app/internal/authflowadapter/flow_test.go, cli/app/internal/authview/text.go, cli/app/internal/embeddedstartup/start.go, cli/app/internal/oauthadapter/oauth.go, cli/app/internal/onboardingimport/skill_metadata.go, cli/app/internal/onboardingmodel/model.go, cli/app/internal/onboardingready/ready.go, cli/app/internal/runtimeconn/errors.go, cli/app/internal/runtimeconn/errors_test.go, cli/app/internal/servecommand/servecommand.go, cli/app/internal/startupconfig/config.go, cli/app/internal/startupconfig/config_test.go, cli/app/internal/statuscollect/auth.go, cli/app/internal/statuscollect/collect.go, cli/app/internal/statuscollect/environment.go, cli/app/internal/statuscollect/model.go, cli/app/internal/statuscollect/usage.go, cli/app/internal/submissionerror/errors.go, cli/app/internal/submissionerror/errors_test.go, cli/builder/binding_commands.go, cli/builder/serve.go	CLI modules systematically update imports: replace builder/server/* with builder/shared/* for shared contracts and builder/cli/*/serverbridge for bridged APIs; update exported type signatures and delegated function calls to use the new contract/bridge types; preserve observable behavior and wiring while changing import dependencies.
Architecture boundary enforcement shared/architecture/boundary_test.go, docs/dev/decisions.md	New architecture test scans CLI code for disallowed builder/server/* imports, maintains an allowlist for explicit serverbridge exceptions, and fails on violations or stale allowlist entries. Documentation clarifies the new boundary rule.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

---

Possibly Related PRs

• respawn-app/builder#38: Introduces the original EnvAPIKeyPreference and startup-gate helpers (EnsureStartupReady/EvaluateStartupGate) that are relocated to shared/auth in this PR.
• respawn-app/builder#193: Adds the generated-skills flow (RecoveredRootNonEmpty, RecoveredWarning, InspectSkills) that is bridged by serverbridge in this PR.
• respawn-app/builder#145: Modifies the session-lifecycle client wiring in cli/builder that is refactored here to use serverbridge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main objective: enforcing a CLI server import boundary through refactoring.
Linked Issues check	✅ Passed	The PR successfully addresses all coding requirements from issue #135: extracts presentation-safe contracts (auth, LLM, session errors, model types) into shared packages, routes CLI wiring through documented serverbridge packages, and adds architectural boundary tests to prevent future violations.
Out of Scope Changes check	✅ Passed	All changes directly support the stated objective of eliminating CLI→server imports via shared contract extraction and serverbridge composition wiring. Documentation updates are appropriately scoped to reflect the new boundary rules.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch td-009

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

cli/builder/internal/serverbridge/serverbridge.go (1)

30-35: ⚡ Quick win

Use bridge alias types in exported function signatures.

StartServe and NewHeadlessHandlers currently expose serverstartup.* types directly despite defining bridge aliases. Using the alias types keeps the bridge API cleaner and better aligned with the import-boundary goal.

Proposed diff

-func StartServe(ctx context.Context, req serverstartup.Request, authHandler serverstartup.AuthHandler, onboardingHandler serverstartup.OnboardingHandler) (ServeServer, error) {
+func StartServe(ctx context.Context, req StartupRequest, authHandler StartupAuthHandler, onboardingHandler StartupOnboardingHandler) (ServeServer, error) {
 	return serve.Start(ctx, req, authHandler, onboardingHandler)
 }
 
-func NewHeadlessHandlers(lookupEnv func(string) string) (serverstartup.AuthHandler, serverstartup.OnboardingHandler) {
+func NewHeadlessHandlers(lookupEnv func(string) string) (StartupAuthHandler, StartupOnboardingHandler) {
 	return serverstartup.NewHeadlessHandlers(lookupEnv)
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli/builder/internal/serverbridge/serverbridge.go` around lines 30 - 35, The
exported functions StartServe and NewHeadlessHandlers should use the bridge
alias types instead of exposing serverstartup.* types: change the StartServe
signature to accept Request, AuthHandler, OnboardingHandler (alias types) while
still calling serve.Start(ctx, req, authHandler, onboardingHandler) and
returning ServeServer, and change NewHeadlessHandlers to return (AuthHandler,
OnboardingHandler) while invoking and returning
serverstartup.NewHeadlessHandlers(lookupEnv) internally; this keeps the bridge
API consistent while leaving internal calls to serverstartup.* unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cli/builder/internal/serverbridge/serverbridge.go`:
- Around line 30-35: The exported functions StartServe and NewHeadlessHandlers
should use the bridge alias types instead of exposing serverstartup.* types:
change the StartServe signature to accept Request, AuthHandler,
OnboardingHandler (alias types) while still calling serve.Start(ctx, req,
authHandler, onboardingHandler) and returning ServeServer, and change
NewHeadlessHandlers to return (AuthHandler, OnboardingHandler) while invoking
and returning serverstartup.NewHeadlessHandlers(lookupEnv) internally; this
keeps the bridge API consistent while leaving internal calls to serverstartup.*
unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9b4fdae2-a0c5-4b2d-bb8f-050b47693acd

📥 Commits

Reviewing files that changed from the base of the PR and between d785882 and dd2ea58.

📒 Files selected for processing (39)

• cli/app/internal/authcommand/slash.go
• cli/app/internal/authflowadapter/flow.go
• cli/app/internal/authflowadapter/flow_test.go
• cli/app/internal/authview/text.go
• cli/app/internal/embeddedstartup/start.go
• cli/app/internal/oauthadapter/oauth.go
• cli/app/internal/onboardingimport/skill_metadata.go
• cli/app/internal/onboardingmodel/model.go
• cli/app/internal/onboardingready/ready.go
• cli/app/internal/runtimeconn/errors.go
• cli/app/internal/runtimeconn/errors_test.go
• cli/app/internal/servecommand/servecommand.go
• cli/app/internal/serverbridge/contracts_test.go
• cli/app/internal/serverbridge/serverbridge.go
• cli/app/internal/startupconfig/config.go
• cli/app/internal/startupconfig/config_test.go
• cli/app/internal/statuscollect/auth.go
• cli/app/internal/statuscollect/collect.go
• cli/app/internal/statuscollect/environment.go
• cli/app/internal/statuscollect/model.go
• cli/app/internal/statuscollect/usage.go
• cli/app/internal/submissionerror/errors.go
• cli/app/internal/submissionerror/errors_test.go
• cli/builder/binding_commands.go
• cli/builder/internal/serverbridge/serverbridge.go
• cli/builder/serve.go
• docs/dev/decisions.md
• server/auth/gate.go
• server/auth/openai_oauth.go
• server/auth/types.go
• server/llm/errors.go
• server/llm/model_catalog.go
• server/llm/types.go
• server/session/store.go
• shared/architecture/boundary_test.go
• shared/auth/types.go
• shared/llmerrors/errors.go
• shared/modelcontract/types.go
• shared/sessioncontract/errors.go

💤 Files with no reviewable changes (2)

• server/auth/gate.go
• server/auth/openai_oauth.go