Files
narratio/docs/roadmap/implementation.md

526 lines
28 KiB
Markdown

# Workstream 1 Implementation Plan
## Purpose And Status
This document is the executable implementation plan for
[`refactoring.md`](refactoring.md), Workstream 1: **Go baseline, process
supervision, and transcription concurrency**. The refactoring roadmap owns the
approved scope, rationale, and target state; the
[`Audit Decision Register`](audit-decisions.md) records the controlling audit
dispositions. This plan translates that settled direction into six bounded
stages suitable for one `gpt-5.6-terra` coding prompt apiece.
All stages are pending and must be implemented in numeric order. Workstreams 2
and later are out of scope. In particular, this plan routes adapter-owned file
replacement through the existing `internal/fileops` owner but does not perform
the broader FileOps simplification reserved for Workstream 2.
## Settled Implementation Decisions
The following decisions are final for this implementation:
- The maintained language baseline is Go 1.26. The module directive is
`go 1.26.0`; ordinary verification and scheduled CI use the official
`golang:1.26` image, while tag CI uses the current explicitly pinned Go
1.26 patch image, `golang:1.26.7`. The release-workflow documentation check
must assert that same pinned tag. Do not retain a Go 1.25 execution path.
- Go 1.26 is adopted before code relies on `os.Process.WithHandle`. Existing
release semantics, supported build targets, and the rule that tag push—not
asynchronous CI completion—is the synchronous release boundary do not
change.
- `golang.org/x/sync/errgroup` is the only new dependency. Add
`golang.org/x/sync v0.22.0` as a direct dependency when transcription starts
using it, then run `go mod tidy`. Do not add an external subprocess,
worker-pool, filesystem, or error framework.
- `subprocess.Run`, `subprocess.RunRequest`, and `subprocess.RunResult` remain
the adapter-facing contract. Their retained behavior includes environment
allowlisting and overrides, sensitive-value redaction, separate or shared
bounded diagnostics, timeouts and cancellation, process-tree termination,
reaping, exit information, error wrapping, and tool-specific diagnostic
hints.
- Split the existing subprocess implementation inside the current
`internal/adapters/subprocess` package. Do not create a public API, a new
repository package, or an external module. A private execution mechanism
owns process-tree creation, start, wait, interruption, bounded termination,
reaping, disposal, timestamps, exit state, and typed termination outcomes.
The existing `Run` boundary remains the Narratio policy wrapper and owns
request validation, timeout construction, environment selection, command
arguments and working directory, diagnostic paths and modes, bounded and
redacted sinks, public result mapping, diagnostic wording, and hints.
- Represent the mechanism result with a private outcome type and private
reason enum that distinguish: not started/start failure, natural exit,
parent cancellation, deadline expiry, and external abort (currently a
diagnostic-size limit). Preserve the underlying start, wait, context, abort,
and cleanup errors as separate fields so the policy layer can retain
`errors.Is`/`errors.As` behavior and join secondary cleanup failures without
replacing the primary classification.
- The mechanism accepts an already policy-configured `*exec.Cmd` and a generic
`<-chan error` abort signal. It must not import `internal/fileops`, inspect
environment variables or secrets, open diagnostic paths, know diagnostic
byte limits or owners, marshal configuration, or construct Narratio-facing
error text. The policy wrapper must not directly start, wait for, kill, or
reap the command.
- On Windows, continue creating the process suspended, configure a
kill-on-close Job Object, assign the child before resuming its initial
thread, and fail closed on every setup error. Perform assignment inside
`cmd.Process.WithHandle`, converting the callback's `uintptr` to
`windows.Handle` only for `windows.AssignProcessToJobObject`. Capture the
callback's assignment error separately from the error returned by
`WithHandle`. Remove the PID-based `windows.OpenProcess` and its associated
process-handle close; do not change the thread-discovery/resumption or job
disposal behavior.
- Remove `WriteYAMLAtomic` and `WriteFileAtomic` from the subprocess package.
Each of `audita`, `scriptorium`, and `seriatim` owns a small private
`writeGeneratedYAML` helper that marshals its generated configuration with
`gopkg.in/yaml.v3` and installs the bytes through
`fileops.WriteFileAtomic` using `fileops.WorkspaceFileMode`. Fake-adapter
byte placeholders call `fileops.WriteFileAtomic` directly. Do not create a
new generic adapter-I/O package merely to preserve the old wrapper.
- Generated configuration must continue to omit API-key values. Preserve the
current schemas and semantic content; incidental YAML key order and exact
wrapper punctuation are not contracts.
- Replace the transcription channel/worker/first-error implementation with
`errgroup.WithContext` and `SetLimit`. Submit one closure per prepared audio
job, use the errgroup context for every WhisperX call, and return
speaker-qualified adapter, authoritative-output, and JSON-validation errors
directly from that closure. Errgroup cancellation replaces the manual
cancel-on-first-error path.
- The transcription mutex protects successful shared aggregation only. It
must not coordinate errors or adapter execution. Canonical materialization
still begins only after every job succeeds and still occurs in sorted speaker
order. A parent cancellation returns an error wrapping the parent context
error and reports completed versus planned work; it does not need to retain
the current incidental “dispatched” count.
- Tests protect observable outcomes and cross-platform compilation, not the
exact internal helper graph, exact full diagnostic strings, or the literal
syscall sequence. Native Windows behavior must not be claimed where only a
Windows cross-compile is available.
- The original audits and decision records are historical inputs. Do not
rewrite them during implementation. Mark Workstream 1 `Complete` in
`refactoring.md` only after all six stages and final validation succeed.
## Instructions For Every Stage
Before changing files in each stage:
1. Read `docs/development.md`, every document under `docs/policy/`, this plan,
and Workstream 1 plus the dependency and target-state sections of
[`refactoring.md`](refactoring.md).
2. Consult the relevant entries H11, H19, H22, H24, H33, and L01 in
[`audit-decisions.md`](audit-decisions.md). Use
[`library-audit.md`](library-audit.md) only for its subprocess-incubation
rationale; its “extract later” possibility is not current scope.
3. Inspect the current implementation and focused tests named by the stage.
Prefer the codebase knowledge graph for Go discovery and use text search for
YAML, shell, Markdown, literals, and generated configuration.
4. Confirm the worktree state and preserve unrelated changes. Do not begin a
later workstream merely because adjacent code would also benefit from it.
During every stage:
- Keep package ownership and dependency direction consistent with
`docs/policy/architecture.md`. Prefer private types and focused files to a new
architectural layer.
- Keep API keys out of arguments where the adapter contract already uses the
environment, and out of generated configuration, logs, errors, fixtures, and
test output. Ordinary workspace files retain group-owned modes.
- Preserve supported Linux, macOS, and Windows behavior. Use build constraints
for platform-specific process code and do not add a permissive unsupported-
platform fallback.
- Keep tests deterministic, offline, and behavior-focused. Exercise configured
and observed concurrency, retain `errors.Is`/`errors.As` assertions where
meaningful, and avoid freezing complete error wording or private helper
choreography.
- Update canonical current-behavior documentation in the same stage that makes
a behavior current. Roadmap documents may describe future work; canonical
docs may not.
- Run `gofmt` on changed Go files, `go mod tidy` when module state changes,
focused tests while iterating, and `git diff --check` before completing the
stage. Mark a stage `Completed` only after all of its exit criteria pass.
## Stage 1 — Establish The Go 1.26 Baseline
**Status: Completed**
### Goal
Move every maintained build and release surface to Go 1.26 before introducing
Go 1.26-only process APIs.
### Required Work
1. Change the module directive in `go.mod` from `go 1.25.0` to
`go 1.26.0`. Run `GOWORK=off go mod tidy` with Go 1.26 and commit any
resulting `go.mod`/`go.sum` changes. Do not add `x/sync` in this stage; it
becomes a used direct dependency in Stage 5.
2. Update every Go step in `.woodpecker/verify.yml` and
`.woodpecker/shuffle.yml` to `golang:1.26`. Update both Go steps in
`.woodpecker/release.yml` to `golang:1.26.7`. Preserve workflow triggers,
dependency ordering, commands, release-plugin configuration, and the
asynchronous release responsibility boundary.
3. Update `internal/doccheck/doccheck_test.go` so the release workflow contract
expects `golang:1.26.7` for validation and asset construction. Keep this
check semantic: it should still validate the shared candidate checker,
asset-builder delegation, step dependencies, and release-plugin settings.
4. Update Go-module fixture text under `internal/releasecheck` from 1.25 to
1.26 where it represents an otherwise valid synthetic module. Do not change
unrelated decimal values such as model/configuration examples.
5. Add a concise Go 1.26 prerequisite to `docs/development.md` and
`docs/release.md`. In the release document, distinguish the maintained
Go 1.26 baseline from the currently pinned tag-CI patch image without
changing the release procedure or promising that automation waits for CI.
6. Search maintained source, workflows, scripts, canonical docs, and semantic
test fixtures for stale Go 1.25 baseline claims. Historical audit prose may
remain historical; controlling roadmap text already records the approved
Go 1.26 decision.
### Exit Criteria
- `go env GOVERSION` reports a Go 1.26 toolchain and
`GOWORK=off go mod tidy -diff` succeeds.
- `go test ./...`, `go test -race ./...`, `go vet ./...`, and
`go build ./...` succeed.
- `go test ./internal/doccheck` and
`go test ./internal/config -run '^TestExamplesLoadAndValidate$'` succeed.
- The six CGO-disabled release targets build through
`scripts/build-release-assets.sh` into a new temporary directory and the
host version smoke check succeeds.
- All maintained Go execution surfaces resolve to Go 1.26, and
`git diff --check` succeeds.
## Stage 2 — Separate Subprocess Lifecycle Mechanics From Policy
**Status: Completed**
### Goal
Make the process-tree lifecycle an internally reusable, typed mechanism while
keeping Narratio-specific environment, diagnostics, redaction, paths, and
error presentation at the existing adapter-facing boundary.
### Required Work
1. Add `internal/adapters/subprocess/execution.go`. Move the complete command
lifecycle behind one private function that receives a context, an already
configured `*exec.Cmd`, and a generic `<-chan error` abort signal. The
function must:
- record UTC start/completion times and initialize an unknown exit code;
- reject an already-done context without starting a child;
- create the platform-owned process tree;
- apply the bounded stream `WaitDelay` before start;
- start exactly once through the tree owner;
- wait once in a buffered result channel;
- on cancellation, deadline, or abort, perform the existing bounded
graceful/forceful termination and reaping sequence;
- dispose the tree on every path after creation; and
- return a private typed outcome containing its reason, timing, exit code,
and separate start, wait, context, abort, and cleanup errors.
2. Keep platform-specific process ownership in `process_tree_*.go`. Reduce
`process_tree.go` to the owned-tree interface, shared termination intervals,
and narrowly reusable termination/reaping helpers needed by
`execution.go`. The lifecycle mechanism, rather than `Run`, must be the only
caller of `Start`, `cmd.Wait`, tree termination, and `Dispose`.
3. Change the diagnostic-limit notification channel in `diagnostics.go` to the
generic error channel accepted by the mechanism. Continue retaining the
first concrete `*captureLimitError` separately so policy formatting can name
the stream, diagnostic owner, and configured limit after the command has
been reaped. Preserve the exact byte caps, redaction, tail bounds, shared-log
locking, flush, and close behavior.
4. Refactor `Run` in `run.go` into the policy/composition layer. It must still:
- validate the executable and normalize a nil parent context;
- create the optional timeout context;
- build the allowlisted, overridden child environment;
- select sensitive values and open bounded redacting log sinks;
- construct and configure `exec.Cmd` arguments, working directory,
environment, stdout, and stderr;
- call the private execution mechanism exactly once;
- flush diagnostics, map the mechanism outcome to `RunResult`, and format
the existing useful start/exit/timeout/cancellation/capture/cleanup
categories; and
- preserve stderr-tail redaction, log paths, bad-descriptor hints, and error
chains.
5. Keep `RunRequest` and `RunResult` source-compatible for every adapter. Exact
punctuation is not required, but existing callers and behavior tests must
continue to receive useful executable/argument/path/exit context and
`errors.Is`/`errors.As` access to cancellation, deadline, `exec.ExitError`,
capture, and cleanup causes.
6. Reorganize `run_test.go` and `process_tree_supported_test.go` only where the
new boundary makes ownership clearer. Retain public `Run` coverage for
success, environment filtering/override, secrets, separate/shared logs,
size limits, timeouts, cancellation, nonzero exit, hinting, and descendant
cleanup. Add focused private-mechanism tests only for a typed distinction
not already protected through `Run`; do not duplicate the whole suite or
assert private function names.
7. Update `internal/adapters/subprocess/doc.go` and
`docs/internal/adapters.md` to describe the implemented internal
mechanism/policy split. Keep the package internal and do not describe it as
a released reusable library.
### Exit Criteria
- The lifecycle mechanism has no dependency on `fileops`, YAML, environment
selection, secret catalogs/redaction policy, diagnostic paths or modes, or
Narratio-facing error wording.
- `Run` does not directly start, wait, terminate, reap, or dispose a process,
and its public request/result surface is unchanged.
- `go test ./internal/adapters/subprocess` and
`go test -race ./internal/adapters/subprocess` pass on the native supported
host.
- Existing adapter subprocess tests for Audita, Scriptorium, Seriatim, and
Notarius pass without loosening credential or diagnostic assertions.
- `go test ./...`, `go vet ./...`, `go build ./...`, and
`git diff --check` succeed.
## Stage 3 — Use The Stable Windows Process Handle
**Status: Completed**
### Goal
Eliminate PID reopening during Windows Job Object assignment while preserving
the fail-closed suspended-child lifecycle.
### Required Work
1. In `process_tree_windows.go`, keep Job Object creation and
`JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` configuration before child start. Keep
`windows.CREATE_SUSPENDED` on the command.
2. After `cmd.Start`, call `cmd.Process.WithHandle`. Inside its callback, call
`windows.AssignProcessToJobObject(job, windows.Handle(handle))` and store
that assignment error in a callback-local variable visible after
`WithHandle` returns. Treat a `WithHandle` error and an assignment error as
distinct setup failures with accurate context.
3. Delete the `windows.OpenProcess` call, the requested process access-mask
constants, and the matching `windows.CloseHandle` for the reopened process.
Never retain or close the handle supplied to the `WithHandle` callback.
4. Resume the initial thread only after successful assignment. Preserve
`resumeInitialThread`, the suspended process's PID-based thread enumeration,
and the existing thread-handle lifetime.
5. On handle acquisition, assignment, or resume failure, kill and wait for the
suspended child, close the not-yet-owned Job Object, and return an error that
preserves the primary setup cause plus cleanup causes. On success, retain
the Job Object in the tree and preserve its kill-on-close disposal behavior.
6. Do not add a test that scans source text for `OpenProcess` or mocks the
standard library. Rely on Go 1.26 Windows compilation for API/type safety
and the existing supported-host process-tree behavior tests for lifecycle
semantics. If a native Windows runner is unavailable, state that limitation
in the implementation-stage handoff rather than claiming runtime coverage.
### Exit Criteria
- `process_tree_windows.go` uses `os.Process.WithHandle` for Job Object
assignment and no longer reopens the child PID as a process handle.
- CGO-disabled Windows `amd64` and `arm64` builds succeed.
- A Windows test binary for `./internal/adapters/subprocess` cross-compiles with
`go test -c`; it is not executed on a non-Windows host.
- Native-host subprocess tests, race tests, repository tests, vet, build, and
`git diff --check` succeed.
- The stage handoff accurately distinguishes cross-compilation from native
Windows runtime evidence.
## Stage 4 — Remove File Writing From The Subprocess Package
**Status: Completed**
### Goal
Make `internal/adapters/subprocess` exclusively about command execution and
move generated configuration and fake-output persistence to the adapters that
own those files.
### Required Work
1. Add one unexported `writeGeneratedYAML(path string, payload any) error`
helper within each of the `audita`, `scriptorium`, and `seriatim` adapter
packages. A focused `generated_config.go` file is preferred in each package
so production and fake runners share the same tool-owned serialization.
2. In each helper, marshal with `yaml.Marshal`, wrap marshal failures with the
destination path, and install the bytes through
`fileops.WriteFileAtomic(path, data, fileops.WorkspaceFileMode)`. Preserve a
path-bearing write error and the existing group-owned mode. Do not put API
key values into the payload while moving code.
3. Replace every production and fake use of
`subprocess.WriteYAMLAtomic` with its package-local helper. Replace every
fake use of `subprocess.WriteFileAtomic` with a direct
`fileops.WriteFileAtomic` call, keeping the existing operation-specific
caller context for outputs, reports, stdout, and stderr placeholders.
4. Remove `WriteYAMLAtomic` and `WriteFileAtomic` from
`internal/adapters/subprocess/run.go`, along with now-unused YAML,
filesystem, mode, and fileops imports. Update the package comment so it
claims only process execution/supervision and Narratio subprocess policy.
5. Delete the two generic YAML-writer tests from subprocess coverage. Do not
recreate tests for a trivial private wrapper in all three packages. Instead,
retain the existing Audita, Scriptorium, and Seriatim production/fake
adapter tests that read generated configurations, check placeholder files,
and assert that credentials are absent. Add a focused adapter test only if
one migrated output is not already covered at its stable adapter boundary.
6. Search all Go source to prove no caller or reference to the removed
subprocess writers remains. Do not move or simplify `fileops` itself; that
package is the temporary focused replacement owner until Workstream 2.
### Exit Criteria
- The subprocess package imports neither `gopkg.in/yaml.v3` nor
`internal/fileops` and exports no file-writing helper.
- All generated YAML and fake adapter files retain their schemas/content,
workspace mode, overwrite behavior, and contextual failures.
- Audita, Scriptorium, and Seriatim production and fake adapter tests pass,
including credential-absence checks.
- Subprocess tests pass after removing only writer-specific coverage.
- `go test ./...`, `go test -race ./internal/adapters/...`, `go vet ./...`,
`go build ./...`, and `git diff --check` succeed.
## Stage 5 — Replace The Transcription Worker Pool With Errgroup
**Status: Completed**
### Goal
Use the maintained bounded-concurrency primitive for per-speaker WhisperX work
while preserving cancellation, all-or-nothing canonical materialization, and
deterministic results.
### Required Work
1. Add `golang.org/x/sync v0.22.0` as a direct dependency and run
`GOWORK=off go mod tidy`. Import `golang.org/x/sync/errgroup` in
`internal/stage/transcribe.go`.
2. Preserve all pre-concurrency behavior: environment validation, session and
run-layout resolution, prepared-audio discovery, positive/default
concurrency calculation, duplicate speaker rejection, and deterministic job
construction.
3. Replace the explicit `context.WithCancel`, worker count, jobs channel,
`sync.WaitGroup`, dispatch loop, and mutex-protected `firstErr` with:
- `group, groupCtx := errgroup.WithContext(ctx)`;
- `group.SetLimit(concurrency)`; and
- one `group.Go` closure per prepared job, capturing a per-iteration job
value and checking `groupCtx` before beginning expensive work.
4. Each closure calls `env.WhisperX.Transcribe` with `groupCtx`, validates the
authoritative output path, validates the run-local transcript JSON, and
returns immediately on failure. Every returned failure must identify the
speaker; adapter failures should also retain the audio basename, and output
failures should retain the relevant output path. Do not store a shared first
error or call a manual cancel function.
5. On success, build that speaker's metadata locally, then lock only long
enough to add it to `perFile` and increment `completed`. Do not keep a
shared map of output refs: each job's planned run-local path is already in
the immutable job list and can be used after `group.Wait` succeeds.
6. After submissions, call `group.Wait` exactly once. Read `completed` under
the same mutex before constructing any diagnostic. Apply this result order:
- if the parent `ctx.Err()` is non-nil, return a transcribe cancellation
error that reports completed and total jobs and wraps that parent error;
- otherwise, if `group.Wait` returned an error, return its
speaker-qualified failure; and
- otherwise, require `completed == len(jobs)` as an internal completeness
guard.
A closure admitted after errgroup cancellation may return the group context
error without invoking WhisperX; the first originating errgroup error remains
authoritative.
7. Preserve the post-success transaction boundary. Sort speaker IDs, then
materialize each planned run-local output to the canonical transcript path
serially in that order. Preserve ordered outputs, run/canonical path
metadata, per-file metadata, retry/language/timeout metadata, and the rule
that no canonical partial output is installed after a job failure or parent
cancellation.
8. Update `internal/stage/transcribe_test.go` to protect the refactored
outcomes:
- configured concurrency remains the observed maximum and never exceeds the
configured limit;
- a speaker failure is returned with speaker context and cancels/blockades
remaining work without deadlock;
- invalid JSON and authoritative-output mismatch remain speaker-qualified;
- parent cancellation wraps `context.Canceled`, reports completed/planned
context, and installs no partial canonical output;
- an already-canceled context makes no WhisperX request; and
- successful results and metadata remain deterministic by speaker.
Use synchronization channels rather than timing alone where a new failure-
cancellation test needs ordering.
9. Update `docs/internal/stage-transcribe.md` to describe bounded per-job
errgroup execution and all-or-nothing canonical materialization. Keep
`docs/integrations/whisperx.md` implementation-neutral unless a retained
integration contract actually changed.
### Exit Criteria
- `transcribe.go` contains no manual worker channel, worker goroutine loop,
shared first-error slot, or manual stage-cancel function.
- `go.mod` records `golang.org/x/sync v0.22.0` directly and
`GOWORK=off go mod tidy -diff` succeeds.
- `go test ./internal/stage -run '^TestTranscribeStage'` and
`go test -race ./internal/stage -run '^TestTranscribeStage'` pass repeatedly,
including with `-shuffle=on -count=10`.
- `go test ./...`, `go test -race ./...`, `go vet ./...`, `go build ./...`,
and `git diff --check` succeed.
## Stage 6 — Integrated Validation And Workstream Closure
**Status: Pending**
### Goal
Verify the combined baseline, process, adapter-file, and transcription changes
at every maintained repository boundary and close only this workstream.
### Required Work
1. Review the complete Workstream 1 diff against this plan and
`refactoring.md`. Remove obsolete imports, helpers, comments, tests, and
documentation claims introduced solely by the replaced mechanisms. Do not
opportunistically implement Workstream 2 filesystem simplification or
external subprocess-library extraction.
2. Confirm canonical documentation is current:
- `docs/development.md` and `docs/release.md` identify Go 1.26;
- `docs/internal/adapters.md` describes the internal mechanism/policy split
and retained process-tree semantics;
- `docs/internal/stage-transcribe.md` describes bounded per-job execution,
cancellation, deterministic ordering, and all-or-nothing canonical
materialization; and
- integration documents retain accurate external contracts without exposing
unnecessary private implementation detail.
3. Search maintained code and configuration for stale Go 1.25 build-image or
baseline assertions, PID-based Windows process reopening, removed
subprocess writer calls, and the old transcription worker machinery.
Exclude historical audit descriptions and unrelated numeric values from
mechanical rewrites.
4. Run the complete release-candidate-equivalent local validation with Go 1.26:
- `GOWORK=off go mod tidy -diff`;
- `gofmt -l` across tracked Go files, requiring no output;
- `git diff --check` and `git diff --cached --check`;
- `go test -count=1 ./...`;
- `go test -race -count=1 ./...`;
- `go vet ./...`;
- `go build ./...`;
- `go test -count=1 ./internal/doccheck`;
- `go test -count=1 ./internal/config -run '^TestExamplesLoadAndValidate$'`;
and
- `scripts/build-release-assets.sh` with a valid synthetic version and a
fresh absolute temporary output directory, proving all six official
CGO-disabled targets compile and the host smoke binary reports that
version.
5. Run the scheduled-CI stress command
`go test -race -shuffle=on -count=3 ./...`. If a failure is genuinely flaky,
diagnose and correct the retained concurrency contract; do not weaken or
skip the test to close the stage.
6. After every check succeeds, change only Workstream 1's status in
`docs/roadmap/refactoring.md` from `Pending` to `Complete`. Leave all later
workstreams pending and leave historical audit decisions intact. Update the
status of all six stages in this document to `Completed` as implementation
history.
### Exit Criteria
- All commands above succeed from a clean, Go 1.26 repository checkout.
- Linux/macOS native-host evidence and Windows/macOS cross-build evidence are
reported accurately; no unrun native platform test is claimed.
- No API key value appears in generated files, diagnostics, errors, fixtures,
or the diff.
- Workstream 1's consolidated target state is fully implemented, its status is
`Complete`, every later workstream remains `Pending`, and the worktree is
clean after the implementation-stage commit.
## Open Questions
None. The audits, refactoring roadmap, approved Go 1.26 revision, and decisions
above resolve the product, compatibility, dependency, ownership, testing, and
platform choices required to implement this workstream.