Skip to content

Conversation

@Adam-D-Lewis
Copy link
Member

Summary

  • Extract workspace business logic from handlers into internal/service/WorkspaceService
  • Typed errors (ErrNotFound, ValidationError, ConflictError) with clean HTTP status mapping via handleServiceError
  • Handlers become thin HTTP wrappers — parse request, call service, format response
  • Router creates service instance once and passes to handler constructor

Key changes

  • internal/service/workspace.go — 11 methods: List, Get, Create, Delete, GetPixiToml, SavePixiToml, PushVersion, ListVersions, GetVersion, GetVersionFile, ListTags
  • internal/service/errors.go — Typed errors for 404/400/409 mapping
  • internal/service/types.go — CreateRequest, PushRequest, PushResult
  • internal/api/handlers/workspace.go — Refactored to thin wrappers with handleServiceError
  • internal/api/router.go — Creates service.New() and passes to handler

Review fixes applied

  • PushVersion: tag conflict check before side effects (prevents orphaned records)
  • GetPixiToml: os.IsNotExist vs other errors (proper 404 vs 500)
  • handleServiceError: slog.Error for unhandled errors
  • ListTags: returns raw models, handler formats timestamps

Test plan

  • go vet ./...
  • go test ./internal/service/... — 21 unit tests covering all service methods
  • go test ./internal/store/... ./cmd/nebi/...
  • cd frontend && npx tsc --noEmit — no type errors

Move workspace business logic from HTTP handlers into a standalone
service package so the CLI can call the same functions directly for
local ops without an HTTP server.

Extracted methods: List, Get, Create, Delete, GetPixiToml, SavePixiToml,
PushVersion, ListVersions, GetVersion, GetVersionFile, ListTags.

Handlers become thin HTTP wrappers that parse params, call the service,
and map typed errors (ErrNotFound→404, ValidationError→400,
ConflictError→409) to status codes.

Non-extracted methods (Install, Remove, Share, Publish, Rollback) remain
in handlers unchanged for now.
Cover delete behavior (managed removes dir, local preserves dir),
GetWorkspacePath routing, and normalizeEnvName edge cases.
- PushVersion: check tag conflict before file writes and version
  creation to prevent orphaned records on conflict
- GetPixiToml: distinguish os.IsNotExist (→404) from other filesystem
  errors (→500) instead of masking all as not-found
- handleServiceError: log unhandled errors with slog.Error before
  returning generic 500
- ListTags: return raw models.WorkspaceTag from service, move timestamp
  formatting to handler (removes presentation logic from service layer)
21 tests covering Create validation (defaults, invalid source,
local-mode restrictions, path requirements), List filtering (local
returns all, team filters to owner), Get/Delete not-found, PushVersion
tag conflict (with and without force, no orphaned records), pixi.toml
round-trip and error handling (missing file vs permission error),
ListVersions, GetVersion, and ListTags.
@Adam-D-Lewis Adam-D-Lewis requested a review from aktech February 9, 2026 16:42
@aktech aktech changed the base branch from workspace-local-mode to main February 11, 2026 13:10
@aktech aktech merged commit dbf0e8e into main Feb 11, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants