refactor: dedupe Go handler/operation boilerplate (#42, #43, #46, #47, #48) #220

Merged

dries merged 5 commits from fix-42 into main 2026-07-04 19:53:54 +02:00

Conversation 1 Commits 5 Files changed 9 +172 -137

dries commented 2026-07-04 13:46:58 +02:00

Owner

Copy link

Addresses the DUP-GO dedup issues, one commit each:

• #42 — listViaAdapter[T] collapses the identical agents/commands/questions session handlers. Scope reduced from the issue: handleSessionPermissions has since grown auto-approve resurrection logic and handleSessionModels returns a pointer response, so both keep their own bodies.
• #43 — marshalRequest centralises the 8x-repeated fmt.Errorf("marshal request: %w", err) wrapping in opencode/operations.go. The table-driven rewrite of the nine operations was skipped deliberately: they diverge in HTTP verb, retry, and cache-invalidation behaviour.
• #46 — validateStateRequest extracts the shared validateID + resolvePlatformIDForState preamble from the seen/archive/pin handlers.
• #47 — paginateSlice[T] + costDescLess dedupe the offset/limit clamping and cost-descending sort in populateSessionLog / populateProjectLog, with table-driven tests.
• #48 — parseAbsDir extracts the required-and-absolute ?dir= validation from five GET handlers (git, worktree, project-upstream). The suggested resolveWorktreeRoot extraction is obsolete: those handlers now delegate through the hostsvc router.

Not included — already fixed on main:

• #44 — both handleSessions and handleSessionsNotify already share fanOutSessions (internal/server/fanout.go). Issue can be closed as done.

Bonus fix (first commit): initOriginRepo (server) and initRepo (hostsvc/local) inherited os.Environ() for git subprocesses. When the test suite runs inside a git hook (pre-commit exports GIT_DIR/GIT_INDEX_FILE), their git init/remote/add/commit operated on the real repository instead of the temp dir — observed live as a stray init commit by t <t@e> landing on this branch during a commit. Reproduced red (hook-env test run corrupted the repo), green after switching to gitexec.CleanEnv() like every other git-spawning helper.

Verified: go vet ./..., full go test ./... (one unrelated flake: TestEnsureToolPathRecoversBinary races on PATH under full-suite parallelism; passes in isolation and on main), platform-branching check.

Addresses the DUP-GO dedup issues, one commit each: - **#42** — `listViaAdapter[T]` collapses the identical agents/commands/questions session handlers. Scope reduced from the issue: `handleSessionPermissions` has since grown auto-approve resurrection logic and `handleSessionModels` returns a pointer response, so both keep their own bodies. - **#43** — `marshalRequest` centralises the 8x-repeated `fmt.Errorf("marshal request: %w", err)` wrapping in `opencode/operations.go`. The table-driven rewrite of the nine operations was skipped deliberately: they diverge in HTTP verb, retry, and cache-invalidation behaviour. - **#46** — `validateStateRequest` extracts the shared validateID + resolvePlatformIDForState preamble from the seen/archive/pin handlers. - **#47** — `paginateSlice[T]` + `costDescLess` dedupe the offset/limit clamping and cost-descending sort in `populateSessionLog` / `populateProjectLog`, with table-driven tests. - **#48** — `parseAbsDir` extracts the required-and-absolute `?dir=` validation from five GET handlers (git, worktree, project-upstream). The suggested `resolveWorktreeRoot` extraction is obsolete: those handlers now delegate through the hostsvc router. **Not included — already fixed on main:** - **#44** — both `handleSessions` and `handleSessionsNotify` already share `fanOutSessions` (`internal/server/fanout.go`). Issue can be closed as done. **Bonus fix (first commit):** `initOriginRepo` (server) and `initRepo` (hostsvc/local) inherited `os.Environ()` for git subprocesses. When the test suite runs inside a git hook (pre-commit exports `GIT_DIR`/`GIT_INDEX_FILE`), their `git init/remote/add/commit` operated on the *real* repository instead of the temp dir — observed live as a stray `init` commit by `t <t@e>` landing on this branch during a commit. Reproduced red (hook-env test run corrupted the repo), green after switching to `gitexec.CleanEnv()` like every other git-spawning helper. Verified: `go vet ./...`, full `go test ./...` (one unrelated flake: `TestEnsureToolPathRecoversBinary` races on PATH under full-suite parallelism; passes in isolation and on main), platform-branching check.

dries added 6 commits 2026-07-04 13:46:58 +02:00

fix(test): strip git hook env in repo-creating test helpers ... 83d4713279

initOriginRepo (server) and initRepo (hostsvc/local) inherited
os.Environ() for their git subprocesses. When the test suite runs
inside a git hook (pre-commit exports GIT_DIR / GIT_INDEX_FILE), those
vars override cmd.Dir and the helpers' git init/remote/add/commit
operate on the real repository instead of the t.TempDir() repo —
observed as a stray 'init' commit by 't <t@e>' landing on the branch
being committed. Use gitexec.CleanEnv() like every other git-spawning
test helper.

refactor(server): collapse duplicated session list handlers into generic helper ... 7cfba68029

The agents, commands, and questions handlers were structurally
identical 11-line functions. Extract listViaAdapter[T] and reduce each
to a one-liner. Permissions and models keep their own bodies: the
former has auto-approve resurrection logic, the latter returns a
pointer response.

Closes #42

refactor(opencode): extract marshalRequest helper for mutating operations ... b6797aec78

The fmt.Errorf("marshal request: %w", err) wrapping appeared
identically 8 times across the mutating operations. Centralise it in
one marshalRequest helper. The table-driven rewrite suggested in the
issue is skipped deliberately: the nine operations diverge in HTTP verb,
retry, and cache-invalidation behaviour, so a table would obscure more
than it saves.

Closes #43

refactor(server): extract shared validation preamble in state handlers ... 3793111a23

handleSeenSession, handleArchiveSession, and handlePinSession repeated
the same validateID + resolvePlatformIDForState block. Extract
validateStateRequest and call it from all three.

Closes #46

refactor(db): dedupe pagination and cost sort in session/project logs ... 794f5ae8d1

populateSessionLog and populateProjectLog ended with identical
offset/limit clamping blocks and near-identical cost-descending sort
comparators. Extract paginateSlice[T] and costDescLess, with
table-driven tests for both.

Closes #47

refactor(server): extract parseAbsDir for the ?dir= query validation ... 59ce9688b5

The required-and-absolute dir validation block was repeated in five
GET handlers across the git, worktree, and project-upstream files.
Extract parseAbsDir (which also applies normaliseDirParam so all sites
share one code path). The resolveWorktreeRoot extraction suggested in
the issue is obsolete: those handlers now delegate through the hostsvc
router and no longer duplicate ResolveRepoRoot error handling.

Closes #48

dries referenced this pull request 2026-07-04 13:47:20 +02:00

DUP-GO-1: platform fan-out loop duplicated in handleSessions and handleSessionsNotify #44

forgejo-actions commented 2026-07-04 13:56:25 +02:00

Copy link

Coverage ratchet: ✅ pass

Suite	Baseline	This PR	Δ
go	70.30%	70.30%	+0.00	✅
frontend	59.12%	59.12%	+0.00	✅

Tolerance: -0.1%. Baseline stored on gh-pages.

<!-- coverage-ratchet --> ### Coverage ratchet: ✅ pass | Suite | Baseline | This PR | Δ | | |---|---|---|---|---| | go | 70.30% | 70.30% | +0.00 | ✅ | | frontend | 59.12% | 59.12% | +0.00 | ✅ | _Tolerance: -0.1%. Baseline stored on `gh-pages`._

dries added 1 commit 2026-07-04 18:45:54 +02:00

Merge remote-tracking branch 'origin/main' into fix-42 ... b5d578e2b1

# Conflicts:
#	internal/hostsvc/local/host_test.go
#	internal/server/handlers_resolve_targets_test.go

dries force-pushed fix-42 from b5d578e2b1 to d1114a9a65 2026-07-04 19:34:01 +02:00 Compare

dries merged commit d1114a9a65 into main 2026-07-04 19:53:54 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

backend

Go backend code

  

bug

Something is broken or incorrect

  

chore

Maintenance, tooling, or cleanup

  

duplication

Code duplication to be addressed

  

effort:complex

Significant effort or risk

  

effort:medium

Moderate effort required

  

effort:trivial

Quick fix, low complexity

  

enhancement

New feature or capability

  

frontend

TypeScript / React frontend code

  

fullstack

Touches both backend and frontend

  

priority:high

Should be addressed soon

  

ready-for-agent

Agent-grabbable ticket, fully specced

  

refactor

Code restructuring without behaviour change

  

security

Security related

No labels

backend

bug

chore

duplication

effort:complex

effort:medium

effort:trivial

enhancement

frontend

fullstack

priority:high

ready-for-agent

refactor

security

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

dries/ocman!220

No description provided.