Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
created_at: 2026-04-06T04:34:47.705819Z
updated_at: 2026-04-06T15:10:46.721169Z
updated_at: 2026-04-06T23:06:50.566563Z
---

## Summary
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
created_at: 2026-04-06T15:10:46.72203Z
updated_at: 2026-04-06T21:18:48.343649Z
updated_at: 2026-04-06T23:06:50.567126Z
---

## Summary
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
---
provider: manual
pr:
round: 1
created_at: 2026-04-06T12:00:00Z
---

## Summary
- Total: 7
- Resolved: 0
- Unresolved: 7
- Resolved: 7
- Unresolved: 0
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

## Executive Summary

The dominant issue across the API layer is **massive duplication between `httpapi` and `udsapi`**. These two packages implement near-identical handler logic, payload structs, SSE streaming, query parsing, memory operations, workspace operations, and test infrastructure -- differing only in transport binding (TCP vs Unix socket) and a few HTTP-specific features (CORS, static file serving, AI SDK prompt streaming). This copy-paste pattern creates a maintenance burden where every handler change must be applied in two places. The `apisupport` package was created to share some workspace/session utilities, but it covers only ~10% of the duplicated surface area. Beyond duplication, the `udsapi/handlers.go` file is a 1,058-line monolith mixing 14+ handler methods, all payload types, SSE infrastructure, and conversion functions.
The dominant issue across the API layer is **massive duplication between `httpapi` and `udsapi`**. These two packages implement near-identical handler logic, payload structs, SSE streaming, query parsing, memory operations, workspace operations, and test infrastructure -- differing only in transport binding (TCP vs Unix socket) and a few HTTP-specific features (CORS, static file serving, AI SDK prompt streaming). This copy-paste pattern creates a maintenance burden where every handler change must be applied in two places. The `apisupport` package was created to share some workspace/session utilities, but it covers only ~10% of the duplicated surface area. Beyond duplication, the `udsapi/handlers.go` file is a 1,084-line monolith mixing 14+ handler methods, all payload types, SSE infrastructure, and conversion functions.

| Severity | Count |
|----------|-------|
Expand All @@ -28,7 +28,7 @@ The dominant issue across the API layer is **massive duplication between `httpap
| 2 | Identical payload struct definitions duplicated across packages | `httpapi/sessions.go` + `udsapi/handlers.go` | moderate | Single source of truth for all API response shapes |
| 3 | Identical memory handler/helper duplication | `httpapi/memory.go` vs `udsapi/memory.go` | moderate | Eliminates 433 copy-pasted lines |
| 4 | Identical test stub types duplicated in test helpers | `httpapi/helpers_test.go` vs `udsapi/helpers_test.go` | trivial | Reduces test code duplication by ~300 lines |
| 5 | `udsapi/handlers.go` monolith at 1,058 lines | `udsapi/handlers.go` | moderate | Improves navigability and cohesion |
| 5 | `udsapi/handlers.go` monolith at 1,084 lines | `udsapi/handlers.go` | moderate | Improves navigability and cohesion |

---

Expand Down Expand Up @@ -129,7 +129,7 @@ func (h *BaseHandlers) CreateSession(c *gin.Context) { ... }

### P1 -- High

#### F2: `udsapi/handlers.go` Monolith (1,058 lines)
#### F2: `udsapi/handlers.go` Monolith (1,084 lines)

- **Smell**: Large Class / Long File
- **Category**: Bloater
Expand Down Expand Up @@ -497,7 +497,7 @@ Recommended sequence based on impact, effort, and dependency between refactoring
1. **Fix typo**: `defaultReadHeaderTimout` -> `defaultReadHeaderTimeout` in `udsapi/server.go:30`
2. **Remove redundant constant**: Delete `const timeRFC3339Nano = time.RFC3339Nano` in `httpapi/stream.go:19`, use `time.RFC3339Nano` directly (F13)
3. **Extract `nullJSON` constant**: Replace magic `"null"` strings with a named constant (F11)
4. **Split `udsapi/handlers.go`** into cohesive files: `sessions.go`, `agents.go`, `observe.go`, `daemon.go`, `stream.go`, `payloads.go` to match httpapi's organization (F2) -- this is a pure file reorganization with no logic changes
4. **Split `udsapi/handlers.go`** into cohesive files: `sessions.go`, `agents.go`, `observe.go`, `prompt.go`, `daemon.go`, `stream.go`, `payloads.go` to match current httpapi organization where applicable (F2) -- this is a pure file reorganization with no logic changes

### Phase 2: High-Impact Structural Changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

## Executive Summary

Both packages are well-structured for a greenfield alpha, but **`workspace/resolver.go` is the clear hotspot** -- at 1,078 lines it aggregates workspace resolution, caching, cloning, filesystem scanning, name generation, and ID generation into a single file. The most impactful opportunity is extracting the ~180 lines of deep-clone boilerplate into a dedicated helpers file and the ~130 lines of filesystem scanning into its own module. A secondary pattern emerges across both packages: **duplicated `fileSnapshot` types and `snapshotsEqual`/`cloneSnapshots` functions** exist independently in `skills/` and `workspace/`, performing identical work with slightly different struct shapes. Both packages also share an identical `checkContext` helper pattern.
Both packages are well-structured for a greenfield alpha, but **`workspace/resolver.go` is the clear hotspot** -- at 1,069 lines it aggregates workspace resolution, caching, cloning, filesystem scanning, name generation, and ID generation into a single file. The most impactful opportunity is extracting the ~180 lines of deep-clone boilerplate into a dedicated helpers file and the ~130 lines of filesystem scanning into its own module. A secondary pattern emerges across both packages: **duplicated `fileSnapshot` types and `snapshotsEqual`/`cloneSnapshots` functions** exist independently in `skills/` and `workspace/`, performing identical work with slightly different struct shapes. Both packages also share an identical `checkContext` helper pattern.

| Severity | Count |
|----------|-------|
Expand Down Expand Up @@ -99,7 +99,7 @@ func FromPath(path string) (Snapshot, error) { ... }
- **Category**: Bloater / Change Preventer
- **Location**: `internal/workspace/resolver.go:1-1078`
- **Severity**: High
- **Impact**: At 1,078 lines, this file handles: workspace CRUD (`Register`, `Unregister`, `Update`, `Get`), resolution + caching (`Resolve`, `ResolveOrRegister`, cache management), filesystem scanning (`scanWorkspace`, `scanAgentSource`, `scanSkillSource`), agent/skill loading (`loadAgents`, `mergeSkillPaths`), deep cloning (~180 lines of `clone*` functions), ID generation, name generation, path canonicalization, and general utility helpers. Any change to workspace scanning, caching, or CRUD requires navigating a 1K-line file.
- **Impact**: At 1,069 lines, this file handles: workspace CRUD (`Register`, `Unregister`, `Update`, `Get`), resolution + caching (`Resolve`, `ResolveOrRegister`, cache management), filesystem scanning (`scanWorkspace`, `scanAgentSource`, `scanSkillSource`), agent/skill loading (`loadAgents`, `mergeSkillPaths`), deep cloning (~180 lines of `clone*` functions), ID generation, name generation, path canonicalization, and general utility helpers. Any change to workspace scanning, caching, or CRUD requires navigating a 1K-line file.

**Recommended Refactoring**: Extract functions into separate files within the same package

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

## Executive Summary

The three packages are well-structured for an alpha project with clean interface boundaries, good error wrapping, and solid test coverage. The most impactful finding is the **Large Class smell in `global_db.go`** (1,097 lines mixing workspace CRUD, session registry, observability writes, and schema migration), which concentrates four unrelated change reasons in one file. The second systemic issue is **pervasive Validate() boilerplate** duplicated across 10+ domain structs with identical `strings.TrimSpace` + switch patterns. A third area is **cross-package duplication of atomic-write logic and test helpers** (`testContext`, `equalStringSlices`, `atomicWriteFile`).
The three packages are well-structured for an alpha project with clean interface boundaries, good error wrapping, and solid test coverage. The most impactful finding is the **Large Class smell in `global_db.go`** (1,099 lines mixing workspace CRUD, session registry, observability writes, and schema migration), which concentrates four unrelated change reasons in one file. The second systemic issue is **pervasive Validate() boilerplate** duplicated across 10+ domain structs with identical `strings.TrimSpace` + switch patterns. A third area is **cross-package duplication of atomic-write logic and test helpers** (`testContext`, `equalStringSlices`, `atomicWriteFile`).

| Severity | Count |
|----------|-------|
Expand Down Expand Up @@ -40,7 +40,7 @@ The three packages are well-structured for an alpha project with clean interface
- **Category**: Bloater + Change Preventer
- **Location**: `internal/store/global_db.go:1-1097`
- **Severity**: High
- **Impact**: Four unrelated change reasons in one 1,097-line file: (1) workspace CRUD, (2) session registry, (3) event summary / token stats observability writes, (4) permission audit log. Any workspace schema change, session lifecycle change, or observability addition modifies this file.
- **Impact**: Four unrelated change reasons in one 1,099-line file: (1) workspace CRUD, (2) session registry, (3) event summary / token stats observability writes, (4) permission audit log. Any workspace schema change, session lifecycle change, or observability addition modifies this file.

**Current Code** (simplified):
```go
Expand Down Expand Up @@ -82,7 +82,7 @@ internal/store/

All methods remain on `*GlobalDB` -- this is a file-level split, not a type decomposition. Each file groups one cohesive responsibility.

**Rationale**: Fowler's Divergent Change: "When you look at a piece of code and decide whether it needs to change for one reason or another, you should be able to focus on the code that changes for that reason." Splitting by file within the same package preserves the Go interface satisfaction while reducing cognitive load from 1,097 to ~200-250 lines per file.
**Rationale**: Fowler's Divergent Change: "When you look at a piece of code and decide whether it needs to change for one reason or another, you should be able to focus on the code that changes for that reason." Splitting by file within the same package preserves the Go interface satisfaction while reducing cognitive load from 1,099 to ~200-250 lines per file.

---

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ The single biggest finding across the entire codebase. `httpapi` and `udsapi` sh
|------|-------|---------|-----------------|
| `daemon/daemon.go` | 1,495 | daemon | boot, dream, orphan cleanup, boundary checks, process utils |
| `session/manager.go` | 1,205 | session | lifecycle, workspace, prompting, recording, cleanup |
| `store/global_db.go` | 1,097 | store | workspace CRUD, session registry, observability, permissions |
| `workspace/resolver.go` | 1,078 | workspace | CRUD, resolution, caching, scanning, cloning, ID gen |
| `udsapi/handlers.go` | 1,058 | udsapi | all handler methods, payload types, SSE, parsers, conversions |
| `store/global_db.go` | 1,099 | store | workspace CRUD, session registry, observability, permissions |
| `workspace/resolver.go` | 1,069 | workspace | CRUD, resolution, caching, scanning, cloning, ID gen |
| `udsapi/handlers.go` | 1,084 | udsapi | all handler methods, payload types, SSE, parsers, conversions |

**Recommendation**: File-level splits within each package (no API changes). Each file already has clear responsibility clusters that can be separated.

Expand All @@ -77,7 +77,7 @@ Functions that appear in 2+ packages with identical or near-identical implementa
| `firstNonEmpty` | session, cli | ~12 |
| `fileSnapshot` + `snapshotsEqual` + clone | skills, workspace | ~60 |
| `testContext(t)` | 7 packages (store, observe, memory, acp, session, daemon, cli) | ~35 |
| `equalStringSlices` | store, observe, daemon | ~30 |
| `equalStringSlices` / `equalStrings` | store, observe, daemon | ~30 |
| `checkContext` | skills, workspace | ~10 |
| `normalizeAbsolutePath` / `expandUserPath` | daemon, config | ~40 |
| **Total duplicated** | | **~363 lines** |
Expand Down Expand Up @@ -116,12 +116,12 @@ Functions that appear in 2+ packages with identical or near-identical implementa
| 1 | Extract shared `apicore/` from httpapi + udsapi (~900 lines dedup) | significant | eliminates shotgun surgery on every handler change | [API](./20260406-api-layer.md) |
| 2 | Split `daemon.go` (1,495 lines) into file-level units | moderate | 5+ responsibility clusters separated, zero API change | [Infra](./20260406-config-daemon-cli.md) |
| 3 | Split `manager.go` (1,205 lines) into lifecycle/prompt/workspace files | moderate | improves navigability, separates concerns | [Core](./20260406-core-session-acp.md) |
| 4 | Split `global_db.go` (1,097 lines) by data domain | moderate | 4 unrelated change reasons isolated | [Storage](./20260406-storage-observe-memory.md) |
| 4 | Split `global_db.go` (1,099 lines) by data domain | moderate | 4 unrelated change reasons isolated | [Storage](./20260406-storage-observe-memory.md) |
| 5 | Extract duplicated process utils to `internal/procutil/` | trivial | eliminates divergent critical process utilities | [Infra](./20260406-config-daemon-cli.md) |
| 6 | Deduplicate Create/Resume activation sequence | moderate | eliminates ~50 lines of copy-paste in session lifecycle | [Core](./20260406-core-session-acp.md) |
| 7 | Extract shared `atomicWriteFile` to `internal/fileutil/` | trivial | fixes latent bug (one calls Sync, other doesn't) | [Storage](./20260406-storage-observe-memory.md) |
| 8 | Extract `processSkill` method in skills registry | moderate | eliminates 3x duplicated load-verify-overlay loop | [New](./20260406-skills-workspace.md) |
| 9 | Split `workspace/resolver.go` (1,078 lines) into focused files | moderate | CRUD/resolution/scanning/cloning separated | [New](./20260406-skills-workspace.md) |
| 9 | Split `workspace/resolver.go` (1,069 lines) into focused files | moderate | CRUD/resolution/scanning/cloning separated | [New](./20260406-skills-workspace.md) |
| 10 | Consolidate test infrastructure into `internal/apitest/` + `internal/testutil/` | trivial | eliminates ~2,000 lines of duplicated test code | [API](./20260406-api-layer.md) + [Storage](./20260406-storage-observe-memory.md) |

---
Expand All @@ -133,8 +133,8 @@ Functions that appear in 2+ packages with identical or near-identical implementa
These are safe, mechanical changes with zero API impact:

1. Extract `processAlive` + `signalProcess` to `internal/procutil/`
2. Extract `atomicWriteFile` to `internal/fileutil/` (fixes subtle Sync divergence)
3. Extract `testContext` + `equalStringSlices` to `internal/testutil/`
2. Extract `atomicWriteFile` to `internal/fileutil/` (adds the missing `Sync` behavior to the `memory/store.go` variant)
3. Extract `testContext` + the duplicated string-slice comparison helpers to `internal/testutil/`
4. Consolidate `userAgentsSkillsDir` into `config/home.go`
5. Merge `cleanupFailedCreate` / `cleanupFailedResume` into `cleanupFailedStart(sessionDir, ...)`
6. Extract `processSkill` method in skills registry (3x loop dedup)
Expand All @@ -147,12 +147,13 @@ These are safe, mechanical changes with zero API impact:

All are file-level reorganizations within the same package:

1. Split `daemon/daemon.go` -> `daemon.go`, `boot.go`, `dream.go`, `orphan.go`, `boundary.go`
2. Split `session/manager.go` -> `manager.go`, `manager_lifecycle.go`, `manager_prompt.go`, `manager_workspace.go`
3. Split `store/global_db.go` -> `global_db.go`, `global_db_workspace.go`, `global_db_session.go`, `global_db_observe.go`
1. Split `daemon/daemon.go` -> `daemon.go`, `boot.go`, `dream.go`, `orphan.go`, `boundary.go`, `notifier.go`
2. Split `session/manager.go` -> `manager.go`, `manager_lifecycle.go`, `manager_prompt.go`, `manager_workspace.go`, `manager_helpers.go`
3. Split `store/global_db.go` -> `global_db.go`, `global_db_workspace.go`, `global_db_session.go`, `global_db_observe.go`, `global_db_permission.go`
4. Split `workspace/resolver.go` -> `resolver.go`, `resolver_crud.go`, `scanner.go`, `clone.go`, `helpers.go`
5. Split `udsapi/handlers.go` -> `sessions.go`, `agents.go`, `observe.go`, `stream.go`, `payloads.go` (matching httpapi layout)
5. Split `udsapi/handlers.go` -> `sessions.go`, `agents.go`, `observe.go`, `prompt.go`, `stream.go`, `daemon.go`, `payloads.go` (matching current httpapi layout where applicable)
6. Split `store/schema.go` -> `schema.go`, `sqlite.go`, `migrate_workspace.go`
7. Split `store/store.go` -> `types.go`, `store.go`, `sql_helpers.go`

### Phase 3: Structural Deduplication (significant effort, high impact -- ~1-2 days)

Expand Down
9 changes: 9 additions & 0 deletions .compozy/tasks/_archived/20260407-030550-refac/_meta.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
created_at: 2026-04-06T23:06:32.101689Z
updated_at: 2026-04-07T03:05:50.12155Z
---

## Summary
- Total: 4
- Completed: 4
- Pending: 0
10 changes: 10 additions & 0 deletions .compozy/tasks/_archived/20260407-030550-refac/_tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Codebase Refactoring — Task List

## Tasks

| # | Title | Status | Complexity | Dependencies |
|---|-------|--------|------------|--------------|
| 01 | Utility packages + inline quick wins | completed | high | — |
| 02 | File-level splits (all bloated files) | completed | high | task_01 |
| 03 | API layer consolidation (apicore + apitest) | completed | critical | task_02 |
| 04 | Domain-level deduplication | completed | high | task_02 |
Loading