Skip to content

Commit 5d438b6

Browse files
refactor(workspace): type workspace responses
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
1 parent 3fbef9e commit 5d438b6

3 files changed

Lines changed: 58 additions & 14 deletions

File tree

internal/mcp/handler_workspace.go

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// @index MCP handlers for namespace workspace uploads, listing, and deletion.
12
package mcp
23

34
import (
@@ -15,33 +16,41 @@ import (
1516
wsvc "github.com/tae2089/code-context-graph/internal/workspace"
1617
)
1718

19+
// workspaceFileResult summarizes one uploaded workspace file.
20+
// @intent preserve a stable per-file DTO for workspace upload responses.
1821
type workspaceFileResult struct {
1922
Namespace string `json:"namespace"`
2023
Workspace string `json:"workspace"`
2124
FilePath string `json:"file_path"`
2225
Size int `json:"size,omitempty"`
2326
}
2427

28+
// workspaceUploadResponse is the typed wire payload for uploadFile and uploadFiles.
29+
// @intent preserve a stable confirmation envelope for single and bulk workspace uploads.
2530
type workspaceUploadResponse struct {
26-
Status string `json:"status"`
27-
Namespace string `json:"namespace,omitempty"`
28-
Workspace string `json:"workspace,omitempty"`
29-
FilePath string `json:"file_path,omitempty"`
30-
Size int `json:"size,omitempty"`
31-
Uploaded int `json:"uploaded,omitempty"`
31+
Status string `json:"status"`
32+
Namespace string `json:"namespace,omitempty"`
33+
Workspace string `json:"workspace,omitempty"`
34+
FilePath string `json:"file_path,omitempty"`
35+
Size int `json:"size,omitempty"`
36+
Uploaded int `json:"uploaded,omitempty"`
3237
Files []workspaceFileResult `json:"files,omitempty"`
3338
}
3439

40+
// workspaceDeleteResponse is the typed wire payload for deleteFile and deleteWorkspace.
41+
// @intent preserve a stable confirmation envelope for workspace deletion operations.
3542
type workspaceDeleteResponse struct {
3643
Status string `json:"status"`
3744
Namespace string `json:"namespace"`
3845
Workspace string `json:"workspace"`
3946
FilePath string `json:"file_path,omitempty"`
4047
}
4148

49+
// workspaceListResponse is the typed wire payload for listWorkspaces and listFiles.
50+
// @intent preserve a stable paged list envelope while omitting irrelevant legacy fields.
4251
type workspaceListResponse struct {
43-
Namespaces []string `json:"namespaces"`
44-
Files []string `json:"files"`
52+
Namespaces []string `json:"namespaces,omitempty"`
53+
Files []string `json:"files,omitempty"`
4554
Items []string `json:"items"`
4655
Count int `json:"count"`
4756
Pagination paging.Page `json:"pagination"`

internal/mcp/handler_workspace_test.go

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,17 @@ func TestListWorkspaces_Empty(t *testing.T) {
213213
if err := json.Unmarshal([]byte(text), &resp); err != nil {
214214
t.Fatalf("failed to parse response: %v", err)
215215
}
216-
workspaces := resp["namespaces"].([]any)
217-
if len(workspaces) != 0 {
218-
t.Errorf("expected empty list, got %v", workspaces)
216+
if _, ok := resp["files"]; ok {
217+
t.Fatalf("expected files field to be omitted, got %v", resp["files"])
218+
}
219+
if items := resp["items"].([]any); len(items) != 0 {
220+
t.Errorf("expected empty items list, got %v", items)
221+
}
222+
if count := resp["count"].(float64); count != 0 {
223+
t.Errorf("expected count 0, got %v", count)
224+
}
225+
if _, ok := resp["pagination"].(map[string]any); !ok {
226+
t.Fatal("expected pagination object")
219227
}
220228
}
221229

@@ -293,6 +301,9 @@ func TestListFiles_Basic(t *testing.T) {
293301
if err := json.Unmarshal([]byte(text), &resp); err != nil {
294302
t.Fatalf("failed to parse response: %v", err)
295303
}
304+
if _, ok := resp["namespaces"]; ok {
305+
t.Fatalf("expected namespaces field to be omitted, got %v", resp["namespaces"])
306+
}
296307
files := resp["files"].([]any)
297308
if len(files) != 2 {
298309
t.Errorf("expected 2 files, got %d: %v", len(files), files)
@@ -335,10 +346,10 @@ func TestListFiles_Pagination(t *testing.T) {
335346
func TestListWorkspaceAndFiles_InvalidPagination(t *testing.T) {
336347
h, _ := workspaceHandlers(t)
337348
for name, req := range map[string]mcp.CallToolRequest{
338-
"workspaces limit": makeCallToolRequest(t, map[string]any{"limit": 0}),
349+
"workspaces limit": makeCallToolRequest(t, map[string]any{"limit": 0}),
339350
"workspaces offset": makeCallToolRequest(t, map[string]any{"offset": -1}),
340-
"files limit": makeCallToolRequest(t, map[string]any{"workspace": "svc", "limit": 0}),
341-
"files offset": makeCallToolRequest(t, map[string]any{"workspace": "svc", "offset": -1}),
351+
"files limit": makeCallToolRequest(t, map[string]any{"workspace": "svc", "limit": 0}),
352+
"files offset": makeCallToolRequest(t, map[string]any{"workspace": "svc", "offset": -1}),
342353
} {
343354
t.Run(name, func(t *testing.T) {
344355
var result *mcp.CallToolResult

internal/workspace/workspace.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,8 @@ type ValidationError struct {
206206
}
207207

208208
// Error returns the validation message.
209+
// @intent expose the caller-fixable validation message without wrapping it in transport-specific formatting.
210+
// @return returns the original validation message stored on the error.
209211
func (e *ValidationError) Error() string { return e.msg }
210212

211213
// IsValidationError reports whether err is a ValidationError.
@@ -215,6 +217,8 @@ func IsValidationError(err error) bool {
215217
return errors.As(err, &v)
216218
}
217219

220+
// newValidationError constructs a ValidationError for caller-fixable workspace input issues.
221+
// @intent centralize creation of typed validation failures so handlers can recognize them consistently.
218222
func newValidationError(msg string) *ValidationError { return &ValidationError{msg: msg} }
219223

220224
// UploadFile validates, decodes, and atomically writes a single workspace file.
@@ -232,12 +236,19 @@ func (s *Service) UploadFile(req UploadRequest) (*UploadResult, error) {
232236
return &UploadResult{Namespace: req.Namespace, FilePath: req.FilePath, Size: len(prepared.decoded)}, nil
233237
}
234238

239+
// preparedUpload holds validated upload data before it is written to disk.
240+
// @intent split validation/decoding from filesystem mutation so bulk uploads can validate before committing files.
235241
type preparedUpload struct {
236242
req UploadRequest
237243
decoded []byte
238244
target string
239245
}
240246

247+
// prepareUpload validates one upload request, decodes its content, and resolves its destination path.
248+
// @intent prepare a single upload for later commit while enforcing per-file and aggregate decoded-size limits.
249+
// @param alreadyDecoded tracks the total decoded bytes accepted earlier in the same bulk request.
250+
// @return returns a preparedUpload ready for commit when validation succeeds.
251+
// @domainRule rejects missing fields, invalid paths, invalid base64, oversized files, and oversized aggregate payloads.
241252
func (s *Service) prepareUpload(req UploadRequest, alreadyDecoded int) (*preparedUpload, error) {
242253
limits := s.limitsOrDefault()
243254
if req.Namespace == "" || req.FilePath == "" || req.ContentBase64 == "" {
@@ -263,6 +274,9 @@ func (s *Service) prepareUpload(req UploadRequest, alreadyDecoded int) (*prepare
263274
return &preparedUpload{req: req, decoded: decoded, target: target}, nil
264275
}
265276

277+
// commitPrepared creates parent directories, revalidates the path, and atomically writes one prepared upload.
278+
// @intent separate filesystem mutation from validation so bulk uploads can fail fast before writing any file.
279+
// @sideEffect creates directories and writes the target file atomically.
266280
func (s *Service) commitPrepared(p *preparedUpload) error {
267281
if err := os.MkdirAll(filepath.Dir(p.target), 0o755); err != nil {
268282
return fmt.Errorf("create directory: %w", err)
@@ -276,6 +290,9 @@ func (s *Service) commitPrepared(p *preparedUpload) error {
276290
return nil
277291
}
278292

293+
// limitsOrDefault returns the configured limits with zero values replaced by workspace defaults.
294+
// @intent ensure upload validation always runs with concrete byte ceilings even when callers omit custom limits.
295+
// @return returns a Limits value whose file, request, and aggregate byte caps are all populated.
279296
func (s *Service) limitsOrDefault() Limits {
280297
l := s.Limits
281298
if l.MaxFileBytes == 0 {
@@ -306,7 +323,14 @@ type BulkEntryError struct {
306323
Err error
307324
}
308325

326+
// Error formats the failing bulk entry index with the underlying error text.
327+
// @intent preserve a user-facing error string that points callers to the exact bad entry.
328+
// @return returns an error string prefixed with the failing entry index.
309329
func (e *BulkEntryError) Error() string { return fmt.Sprintf("entry %d: %v", e.Index, e.Err) }
330+
331+
// Unwrap returns the underlying entry error.
332+
// @intent allow callers to inspect the original validation or filesystem failure that broke a bulk upload.
333+
// @return returns the underlying entry error for errors.Is and errors.As checks.
310334
func (e *BulkEntryError) Unwrap() error { return e.Err }
311335

312336
// UploadFiles parses a bulk upload JSON payload, validates every entry, and writes them sequentially.

0 commit comments

Comments
 (0)