chore: tech debt sweep — #96, #97, #98, #99, #102

[dean0x]

Summary

Post-v0.6.0 tech debt cleanup. Closes 5 issues; 3 were already resolved, 2 required code changes.

• tech-debt: WorkerRepository missing Zod row validation schema #96 (WorkerRepository Zod validation): Already implemented — closed with comment
• tech-debt: WorkerRepository missing operationErrorHandler #97 (WorkerRepository operationErrorHandler): Already implemented — closed with comment
• tech-debt: worker-repository.test.ts not excluded from test:implementations #99 (worker-repository.test.ts grouping): By design — closed with comment
• tech-debt: EventDrivenWorkerPool.spawn() at 99 lines — extract post-spawn setup #98: Extract registerWorker() from spawn() — reduces method from ~99 to ~65 lines
• Extract exitOnError CLI helper to reduce boilerplate #102: Extract exitOnError/exitOnNull CLI helpers — eliminates 13 repeated guard blocks across status.ts, logs.ts, schedule.ts (net -41 lines)

Also updated #31 tech debt backlog: marked 3 stale/fixed entries, updated open count from 14 → 4.

Closes #96, closes #97, closes #98, closes #99, closes #102

Test plan

• npm run build — clean typecheck
• npm run test:implementations — 314 tests pass (covers tech-debt: EventDrivenWorkerPool.spawn() at 99 lines — extract post-spawn setup #98)
• npm run test:cli — 157 tests pass (covers Extract exitOnError CLI helper to reduce boilerplate #102)
• npx biome check — clean
• Snyk SAST — 0 issues

[greptile-apps]

Confidence Score: 5/5

• Safe to merge — all changes are non-breaking refactors with identical runtime behaviour verified by passing test suites.
• All error messages and spinner stop messages are preserved exactly from the original code. The registerWorker extraction preserves the rollback sequence for Edge Case J. TypeScript typechecks cleanly (npm run build) and all 471 tests pass. The only observation is a minor style inconsistency (discarded exitOnNull return in logs.ts) with no functional impact.
• No files require special attention.

Important Files Changed

Filename	Overview
src/cli/services.ts	Introduces exitOnError and exitOnNull guard helpers that eliminate repeated error-handling boilerplate. Clean implementation — the generic signature correctly constrains E = Error, and process.exit(1) (typed never) ensures sound return-type inference. withReadOnlyContext and withServices are correctly simplified using these helpers.
src/cli/commands/logs.ts	Guard pattern applied correctly; error messages and spinner stop messages are preserved from the original. Minor style inconsistency: the narrowed return value of exitOnNull on line 17 is discarded (the task value is only needed as an existence guard here), whereas status.ts captures and uses the narrowed value.
src/cli/commands/status.ts	Correctly uses both exitOnError and exitOnNull; the narrowed task value is captured and used for display. Spinner stop messages ('Failed', 'Not found') match the originals via exitOnNull's default stopMsg = 'Not found'. Logic for the listing path (findAll) is equivalent — no behavioral regression.
src/cli/commands/schedule.ts	All 5 schedule sub-commands cleanly refactored. undefined is correctly passed for the spinner in scheduleCreate, scheduleList, scheduleGet, scheduleCancel, schedulePause, and scheduleResume because the spinner is stopped (.stop('Ready')) before these functions are called from handleScheduleCommand. Return types added to previously untyped private functions. Error messages match originals exactly.
src/implementations/event-driven-worker-pool.ts	Clean extraction of registerWorker(). The rollback sequence (SIGTERM → delete from both maps → return error) is correctly preserved; Edge Case J behaviour is unchanged. spawn() is reduced from ~99 to ~65 lines with no behavioral difference. startedAt: Date.now() being called twice (once for WorkerState, once for DB registration) was pre-existing, not introduced here.

Sequence Diagram

sequenceDiagram
    participant CLI as CLI Command<br/>(logs / status / schedule)
    participant SVC as services.ts<br/>(exitOnError / exitOnNull)
    participant REPO as Repository
    participant UI as ui.*
    participant PROC as process

    CLI->>SVC: withReadOnlyContext(s) / withServices(s)
    SVC->>SVC: exitOnError(createReadOnlyContext(), s, prefix)
    alt result.ok == false
        SVC->>UI: ui.error(prefix + message)
        SVC->>PROC: process.exit(1)
    else result.ok == true
        SVC-->>CLI: ctx / services (unwrapped T)
    end

    CLI->>REPO: findById / findAll / etc.
    CLI->>SVC: exitOnError(result, s, prefix)
    alt result.ok == false
        SVC->>UI: ui.error(prefix + message)
        SVC->>PROC: process.exit(1)
    else result.ok == true
        SVC-->>CLI: value: T
    end

    opt value may be null
        CLI->>SVC: exitOnNull(value, s, msg)
        alt value == null
            SVC->>UI: s.stop(stopMsg)
            SVC->>UI: ui.error(msg)
            SVC->>PROC: process.exit(1)
        else value != null
            SVC-->>CLI: value: T (narrowed, non-null)
        end
    end

    CLI->>UI: ui.success / ui.info / ui.note
    CLI->>PROC: process.exit(0)

Loading

_{Last reviewed commit: "style(cli): dogfood ..."}

[greptile-apps]

Discarded return value of exitOnNull

The narrowed, non-null Task returned by exitOnNull is discarded here. After this line, task is still typed Task | null (from the exitOnError call above). This differs from the pattern in status.ts where the result is always captured:

const found = exitOnError(result, s, 'Failed to get task status');
const task = exitOnNull(found, s, `Failed to get task status: ...`);

Since the task value isn't used beyond this existence check, consider clarifying intent — for example, by using a clearer variable name like _existingTask or adding a comment:

Suggested change

	exitOnNull(task, s, `Failed to get task logs: ${taskNotFound(taskId).message}`);
	exitOnNull(task, s, `Failed to get task logs: ${taskNotFound(taskId).message}`); // guard: validate task exists

Or capture the narrowed value for consistency (even if unused):

const _task = exitOnNull(task, s, `Failed to get task logs: ${taskNotFound(taskId).message}`);

This makes it clear that the call is solely an existence guard and not a logic oversight.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}