diff --git a/docs/roadmap/fileops.md b/docs/roadmap/fileops.md index 3bf57b6..77b82b9 100644 --- a/docs/roadmap/fileops.md +++ b/docs/roadmap/fileops.md @@ -2,7 +2,7 @@ ## Status -Proposed feature roadmap. This document defines the desired filesystem policy +Accepted feature roadmap. This document defines the desired filesystem policy and target state. It is not an implementation plan and does not describe implemented behavior. It refines the filesystem conclusions in [`audit.md`](audit.md). diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index fbacfae..e685fe8 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,525 +1,671 @@ -# Workstream 1 Implementation Plan +# Workstream 2 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. +[`refactoring.md`](refactoring.md), Workstream 2: **Filesystem simplification +and restore correctness**. The accepted [`File Operations Simplification`](fileops.md) +roadmap owns the filesystem threat model, policy choices, compatibility intent, +and target state. The [`Audit Decision Register`](audit-decisions.md) records +the controlling audit dispositions. This plan translates that settled direction +into eight 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. +All stages are pending and must be implemented in numeric order. Workstream 1 +is complete. Workstreams 3 and later are out of scope: in particular, do not +purge deprecated state/configuration readers, eliminate run manifests, change +analyze checkpointing, decompose publish, or add the final cleanup stage while +performing this plan. ## 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. +- Narratio operates in a trusted, session-locked, group-owned workspace. It + does not defend ordinary filesystem access against a hostile concurrent + mutator. API-key material remains sensitive, and destructive cleanup and + restore remain explicitly root-scoped. +- Use only the Go 1.26 standard library for the filesystem simplification. Do + not add a filesystem, atomic-write, copy, or path-safety dependency. Keep the + retained helpers internal to Narratio; the library audit explicitly rejected + extraction of FileOps. +- Preserve package ownership and the existing acyclic dependency direction. + Ordinary `os`, `io`, and `path/filepath` operations belong beside their domain + callers. `internal/fileops` may retain only shared product policy: workspace + modes, minimal same-directory atomic file replacement, bounded reads, mode + normalization for an externally produced tree, and root-scoped deletion. +- Retain `fileops.WorkspaceDirectoryMode` (`02775` on POSIX) and + `fileops.WorkspaceFileMode` (`0664` on POSIX) as the single discoverable mode + definitions. `EnsureWorkspaceDirectory` must create missing components and + explicitly normalize every component it creates plus the requested directory, + so a restrictive umask cannot defeat group operation. It must not change + ownership. POSIX mode bits remain non-authoritative on Windows. +- Retain atomic visibility for corruption-sensitive single files. The minimal + replacement sequence is: require an existing destination parent, create a + sibling temporary file, stream/write bytes, apply the final mode, close, + perform any caller-supplied pre-rename cancellation check, and replace with + `os.Rename`. Remove the temporary file on every pre-commit failure. Do not + call `File.Sync`, sync a directory, probe sync capabilities, preserve + `ErrDirectorySyncUnsupported`, or claim survival across power loss. +- Preserve the useful atomic replacement entry points used by multiple domains: + byte replacement, write-with-parent-creation, reader/copy replacement with a + SHA-256 result, and validated sibling-temp installation. Rename or consolidate + them where that makes ownership clearer, but do not retain two implementations + of the same replacement sequence or an exported compatibility wrapper with no + production caller. +- Replace `ReadRegularFile` and the general confined-read facade with one small + `ReadFileBounded`-style helper. It uses `os.Open`, a limit-plus-one read, and + returns a clear limit error; it follows ordinary trusted-workspace symlink + semantics. A consumer may inspect `File.Stat`, size, checksum, or decoded + structure when its own external contract requires those properties. Do not + reconstruct a package-wide regular-file/no-follow rule in callers. +- Remove `OpenFileConfined`, `OpenConfinedRegularFile`, + `ValidateConfinedDirectory`, and the general root-walking implementation after + their callers migrate. Configuration imports and ordinary artifacts may use + standard path semantics. The API-key loader is the exception: it keeps its + 8-KiB limit, restrictive POSIX mode checks, regular-file requirement, + symlink rejection, environment precedence, and non-disclosure behavior in + app-owned credential helpers. +- Remote content must be fully written to a temporary file and validated before + it becomes a canonical destination. Domain owners decide what validation is + meaningful: expected size/generation/checksum, non-empty or format parsing, + committed-object identity, or audio/cache identity. Use centralized existing + limits for bounded configuration/control objects and adapter results. Do not + invent a low global media-size cap; a transfer with an authoritative declared + size must reject a size mismatch. Any retained safety-limit error must name + the enforcing adapter/category and the effective numeric limit. +- Keep a single minimal sibling-temp installation operation so audio, restore, + and previous-artifact flows do not duplicate replacement policy. Remove + `DownloadedTempFile`, `DownloadAndInstall`, `DownloadToSiblingTemp`, and any + standalone install surface left without a production caller once their flows + are migrated. +- Cleanup accepts an explicit root and a proper-descendant target, converts the + target to a normalized root-relative name, opens the root with `os.OpenRoot`, + and delegates recursive deletion to `Root.RemoveAll`. A missing target is + success. Exact-file deletion uses `Root.Lstat` and `Root.Remove`, remains + idempotent, and rejects a non-regular leaf. Root deletion, traversal, an + absolute relative name, or an out-of-root target remains an error. Do not keep + custom recursive traversal, sorting, handle chains, or a universal rule that + refuses a symlink which `os.Root` can safely contain. +- Restore planning opens the selected session root and classifies targets by + their normalized session-relative paths. Walk each existing ancestor with + `Root.Lstat`/`Root.OpenRoot`; any ancestor or leaf symlink is a non-regular + conflict. A directory remains a directory conflict. Hash only an opened, + verified regular target beneath that root. `--force` may turn a differing + regular file into a download, but must never turn an ancestor symlink, leaf + symlink, directory, or other non-regular entry into an overwrite. +- Restore execution must repeat the root-relative parent/leaf safety check at + installation time, create missing destination parents with workspace modes, + download to a sibling temporary file, validate the complete object, and only + then install it. Preserve manifest-last ordering, manifest rebasing and + validation, committed size/checksum/generation checks, audio cache identity, + incomplete-restore markers, reports, and safe retry. Do not add hostile-race + inode rechecks. +- Extract keeps Notarius integration validation in the Notarius adapter and + stage. The adapter-reported bundle must remain a proper descendant of the + unique run-local output root and satisfy the current v0.6 bundle contract. + Normalize the validated tree to workspace directory/file modes, require the + unique durable bundle destination to be absent, then use one `os.Rename`. + Do not copy the bundle, retain the pre-promotion tree, or provide a custom + no-replace race guarantee. A destination collision is an ordinary explicit + error. +- The run-local Notarius output root is owned by the extract invocation. After + verifying that scope, remove failed staging content; after success, remove the + now-empty staging root. Receipt and stderr paths remain the diagnostic record. + Never delete an adapter-reported path merely because the adapter returned it. +- Retire tests for syscall ordering, sync failures, directory-promotion hooks, + inode identity races, universal no-follow behavior, and platform capability + errors with the corresponding contract. Retain or add the smallest durable + behavior tests for atomic visibility/cleanup, bounded reads, modes, validated + downloads, cleanup scope, restore symlink conflicts, Notarius rename, and + credential protection. +- Current-behavior documentation changes in the same stage as code. Volatile + modes, paths, limits, and procedures remain in their canonical owners; other + documents summarize and link. Historical audits and release notes remain + historical and must not be rewritten. +- Mark Workstream 2 `Complete` in `refactoring.md` only after all eight stages + and final validation succeed. Do not change another workstream's status. ## 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. + the whole accepted [`fileops.md`](fileops.md) roadmap, and Workstream 2 plus + the dependency and final-target sections of [`refactoring.md`](refactoring.md). +2. Consult only the concise decision-register rows H02, H06, H07, H18, H20, + H21, H29, H45, H46, C04, L02, L05a, and L05c in + [`audit-decisions.md`](audit-decisions.md). The source audits are historical + evidence and need not be reread or edited for implementation. 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. + documentation, literals, build constraints, and stale contract language. 4. Confirm the worktree state and preserve unrelated changes. Do not begin a - later workstream merely because adjacent code would also benefit from it. + later workstream merely because adjacent compatibility, manifest, publish, + cleanup-stage, or configuration code would also benefit from refactoring. 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. +- Keep the repository building and the focused tests passing at the end of the + stage. Temporary internal migration surfaces are permitted only when a later + numbered stage in this plan explicitly removes them; add no deprecation or + compatibility layer. +- Use direct standard-library operations for trusted ordinary data. Keep + adapter contract validation with adapters, stage policy with stages, restore + policy with the application, and canonical path construction with artifacts + and `pathsafe`. +- Preserve group-owned workspace operation. Never put API-key values in command + arguments, configuration, logs, errors, fixtures, manifests, reports, or + ordinary workspace files. Never include secret contents in a test failure. +- Preserve Linux, macOS, and Windows behavior. Platform-specific code is + allowed only if the retained single-file replacement behavior demonstrably + needs it. Cross-compilation establishes compilation, not native runtime + evidence. +- Keep tests deterministic, offline, and behavior-focused. Use `t.TempDir()` + and real filesystem operations. Prefer semantic error fragments or typed + causes over exact complete wording, and delete obsolete mechanism tests in + the same stage that retires their contract. +- Update canonical current-behavior documentation in the stage that changes the + behavior. Roadmap documents may describe future state; canonical docs must + not describe a partially implemented later stage as current. +- Run `gofmt` on changed Go files, 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 +## Stage 1 — Reduce Atomic Replacement, Modes, And Bounded Reads -**Status: Completed** +**Status: Pending** ### Goal -Move every maintained build and release surface to Go 1.26 before introducing -Go 1.26-only process APIs. +Establish the small shared filesystem foundation that later stages will use, +while removing power-loss durability and test-hook machinery from single-file +replacement. ### 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. +1. Rewrite `internal/fileops/fileops.go` around one private reader-to-sibling- + temp replacement implementation. Preserve `ReplaceFileOptions.Mode` and + `BeforeRename`, context/caller error wrapping, cleanup-error joining, checksum + calculation, destination-parent ownership, and replacement of an existing + regular destination. Remove `temporaryFile`, `atomicReplacementOperations`, + system operation tables, injected operation variants, `Sync` calls, and + directory-sync calls from the file replacement path. +2. Make the final sequence explicit in code and package documentation: create + sibling temp, stream bytes, apply mode, close, run the optional pre-rename + check, and `os.Rename`. If creation, writing, chmod, close, or the pre-rename + check fails, remove the temp and retain the old destination. If rename fails, + retain the old destination and remove the temp. A successful rename returns + success without an `fsync` claim. +3. Consolidate the public internal entry points so byte writes, reader writes, + copies, checksum-returning copies, and sibling-temp installation share that + implementation. Introduce the final clearly named temp-install function and + migrate existing callers of `InstallDownloadedTempFile` as needed within the + package. Do not preserve an alias after all callers migrate in Stage 3. +4. Rewrite `EnsureWorkspaceDirectory` in `modes.go` with ordinary `os` and + `filepath` operations. Find the nearest existing ancestor, create the missing + suffix, and explicitly apply `WorkspaceDirectoryMode` to each directory that + was created and to the requested directory. Reject an existing non-directory + at the requested path; do not add symlink/race identity machinery. +5. Add a small `ReadFileBounded` implementation that opens the path normally, + reads through `io.LimitReader(limit+1)`, and reports limit overflow without + returning partial content. Reject invalid limits and empty paths. Retain the + limit in the error so adapter/category wrappers can provide unmistakable + context. Leave the old confined read names only until Stage 2 migrates their + callers. +6. Rewrite `internal/fileops/fileops_test.go`, `modes_posix_test.go`, and the + bounded-read tests around observable outcomes: old-or-new content only, + destination replacement, pre-rename cancellation, temp cleanup after each + meaningful pre-commit failure category, checksum correctness, exact/plus-one + bounds, and group modes under a restrictive umask. Delete operation-order, + sync, race-hook, symlink, and private-operation-table assertions whose + guarantees are retired. +7. Update `docs/internal/fileops.md`, the atomic-save section of + `docs/internal/manifest.md`, and the narrow durability wording in + `docs/policy/architecture.md` to describe atomic visibility rather than + crash/power-loss durability. Do not yet describe directory promotion or + cleanup as removed; those change in later stages. + +### Exit Criteria + +- `go test ./internal/fileops ./internal/manifest ./internal/artifacts` succeeds. +- Atomic replacement leaves the prior destination intact and no sibling temp + after a pre-commit failure, and produces the intended mode/content on success. +- The bounded helper accepts exactly the configured limit and rejects limit+1. +- No single-file replacement path calls `Sync` or reports a directory-sync + capability error; directory-promotion-only code may remain until Stage 7. +- `git diff --check` succeeds. + +## Stage 2 — Migrate Ordinary I/O And Isolate Credential Handling + +**Status: Pending** + +### Goal + +Remove universal confined/no-follow access from ordinary trusted-workspace +callers without weakening the one sensitive filesystem boundary: API-key +loading. + +### Required Work + +1. Migrate bounded local-result readers in the Audita, Notarius, Seriatim, and + Scriptorium adapter packages, `internal/artifacts`, `internal/contracts`, + `internal/config/party_resolution.go`, and `internal/stage` to the Stage 1 + bounded helper. Preserve each owner/category's existing centralized limit, + semantic decoding, non-empty checks, checksum rules, and clear adapter-named + error context. Do not centralize adapter-specific limits in FileOps. +2. Replace ordinary uses of `OpenConfinedRegularFile` with direct `os.Open` and + the minimum domain check actually required. Publish sources, artifact + evidence, prepared inputs, and extract references must still be opened before + hashing/copying and checked as regular when their declared artifact contract + requires it; remove `SameFile` and pathname reinspection. Configuration + imports/overlays use normal `os.Open` semantics after existing lexical + normalization, extension, duplicate, and composition checks. +3. Replace `OpenFileConfined` at diagnostic-log and session-lock sites with + explicit parent creation plus `os.OpenFile` and mode normalization owned by + those packages. Preserve lock behavior and subprocess diagnostic bounds, + redaction, close/flush behavior, and workspace modes. Do not change process + supervision. +4. Move credential-only path handling into `internal/app/secrets_env*.go`. + Open and validate the configured secret directory and each candidate without + following a symlink; require regular files, preserve restrictive POSIX mode + validation (and documented Windows ACL responsibility), read at most + `APIKeyFileByteLimit`, and keep existing-environment precedence. A simple + inspection/open sequence is sufficient; remove adversarial replacement hooks + and inode identity rechecks. +5. Remove `ValidateConfinedDirectory`, `ReadRegularFileUnderRoot`, + `ReadRegularFile`, `OpenConfinedRegularFile`, `OpenFileConfined`, and their + tests once production callers are gone. Delete the general confined-open and + confined-read files if no later stage depends on them; keep only narrowly + identified cleanup/download code needed temporarily by Stages 3 and 4. +6. Update `docs/config.md`, `docs/internal/configuration.md`, adapter/integration + documents, `docs/internal/artifacts.md`, `docs/internal/stage-analyze.md`, + `docs/internal/stage-publish.md`, and `docs/internal/workspace.md` where they + currently present universal no-follow/confined reads as application-wide + behavior. Preserve and clearly isolate the stricter API-key contract in + `docs/config.md` and `docs/operations.md`. +7. Adjust focused tests to protect owner-level outcomes rather than the deleted + helper choreography. Retain adapter limit tests, artifact checksum/contract + tests, configuration composition tests, publish-source validation, lock + behavior, subprocess redaction, and credential symlink/mode/limit tests. + Delete tests whose only purpose is universal no-follow or identity-race + resistance. + +### Exit Criteria + +- `go test ./internal/adapters/... ./internal/artifacts ./internal/contracts ./internal/config ./internal/stage ./internal/app` succeeds. +- Ordinary bounded reads follow standard path semantics and retain their + category-specific limits and useful errors. +- API-key files remain bounded, regular, non-symlink, private on POSIX, and + absent from errors and logs. +- No production caller uses the general confined open/read APIs. +- Canonical docs distinguish ordinary trusted-workspace access from credential + and destructive/restore safety boundaries. +- `git diff --check` succeeds. + +## Stage 3 — Validate Downloads Before Canonical Installation + +**Status: Pending** + +### Goal + +Consolidate temporary-download behavior so incomplete or invalid remote content +never becomes a canonical local file, then remove the overbuilt FileOps download +object model. + +### Required Work + +1. Define one straightforward sibling-temp lifecycle used by the affected + domain owners: ensure the destination parent with workspace modes, create a + uniquely named sibling temp, stream through `storage.DownloadTo` or verified + in-memory content, close, validate the complete temp, install through the + Stage 1 temp-install replacement, and remove the temp on every failure. + Keep transport mechanics in `internal/adapters/storage`; keep validation and + destination policy with audio, restore, configuration, or stages. +2. Rework `internal/audio/s3_audio.go` and `cache_identity.go` so a remote audio + object is downloaded to temp and checked against authoritative declared size + plus the existing local-audio requirements before it becomes the spool file. + Reuse cached audio only after its typed remote identity, regular opened file, + exact size, and SHA-256 match; install it from temp only after validation. + Preserve cache miss/hit accounting, checksum records, context cancellation, + and group modes. +3. Rework previous-session hydration in `internal/stage/prepare_previous.go` so + `VerifiedContent` or streamed remote content is checked against every + available declared size/checksum and the existing non-empty artifact rule + before installation. Keep the current legacy/canonical resolution branches; + Workstream 3, not this stage, removes compatibility behavior. +4. Audit `storage.DownloadObjectToTemp` callers used for remote session config, + restore planning, and current-state discovery. Where a control/config object + already has a centralized limit, use `ReadObjectBounded` and parse/validate + bytes directly or enforce that limit while writing temp. A temporary file + used only for comparison may remain system-temp state, but it must be removed + reliably and must not be described as canonical installation. +5. Keep restore's committed-object temp validation behavior working, but defer + its final root-scoped installation redesign to Stage 6. It may use the new + minimal temp lifecycle as an interim caller. +6. Remove `DownloadedTempFile`, `DownloadAndInstall`, + `DownloadToSiblingTemp`, the old confined download installer, redundant + exported install aliases, and their mechanism tests when no production + caller remains. Keep only the single temp-install replacement primitive + selected in Stage 1. +7. Update `docs/internal/storage.md`, `docs/internal/artifacts.md`, + `docs/internal/command-restore.md`, and `docs/operations.md` to state that + transfer and validation precede canonical installation, without duplicating + owner-specific numeric limits. +8. Add or retain focused tests for failed/partial download cleanup, declared + size mismatch, checksum/identity mismatch, invalid configuration or manifest + content, cache corruption, successful install modes, and preservation of an + existing canonical destination after any validation failure. + +### Exit Criteria + +- `go test ./internal/adapters/storage ./internal/audio ./internal/stage ./internal/app ./internal/artifacts` succeeds. +- Audio, previous-session, remote configuration/control, and restore download + paths validate the complete temp or bounded bytes before canonical install. +- A failed transfer or validation does not replace an existing destination and + leaves no owned sibling temp. +- The old downloaded-temp object model and unused install surfaces have no + production callers and are deleted. +- `git diff --check` succeeds. + +## Stage 4 — Replace Custom Cleanup Traversal With `os.Root` + +**Status: Pending** + +### Goal + +Preserve accidental out-of-root deletion protection while deleting custom +recursive traversal, no-follow handle chains, and sorting machinery. + +### Required Work + +1. Rewrite `RemoveAllUnderRoot` to validate non-empty paths, resolve absolute + clean root and target paths, require the target to be a proper descendant, + derive and normalize one root-relative name, open the explicit root with + `os.OpenRoot`, and call `Root.RemoveAll`. Missing targets remain success. + Never pass an absolute path, `.`, `..`, or traversal to a root method. +2. Rewrite `RemoveFileUnderRoot` with the same root/relative validation, + `Root.Lstat`, regular-leaf classification, and `Root.Remove`. Missing is + success; directories, symlinks, and other non-regular leaves are errors. + Keep this exact-file rule because callers intend to clear one managed file, + not because it is a universal workspace policy. +3. Delete `openConfinedCleanupTarget`, filesystem-root walking, + `removeConfinedEntry`, `removeConfinedChildren`, deterministic traversal + sorting, and any general confined-directory code left solely for cleanup. +4. Keep existing application eligibility checks in `cleanup_targets.go`, + `clean.go`, post-publish cleanup, prepare's managed previous-state reset, + remote-session-config temp cleanup, and restore-marker cleanup. Do not move + cleanup out of publish, add a pipeline stage, change cleanup obligations, or + change cache-selection policy; those belong to Workstream 9. +5. Rewrite `internal/fileops/cleanup_test.go` around proper-descendant + rejection, root/self/traversal/outside refusal, nested removal, missing + idempotence, root-contained symlink behavior, and exact-file non-regular + refusal. Retain app-level tests for policy-owned target eligibility. +6. Update the cleanup/confinement sections of `docs/internal/fileops.md`, + `docs/internal/workspace.md`, `docs/operations.md`, and + `docs/troubleshooting.md`. Describe root confinement and eligibility, not a + custom no-follow recursive algorithm. + +### Exit Criteria + +- `go test ./internal/fileops ./internal/app ./internal/stage` succeeds. +- Root, out-of-root, traversal, and overly broad targets are rejected; an + eligible nested target is removed idempotently through `os.Root.RemoveAll`. +- No custom recursive cleanup walker or ancestor-handle chain remains. +- Current explicit and post-publish cleanup behavior is otherwise unchanged. +- `git diff --check` succeeds. + +## Stage 5 — Make Restore Planning Symlink-Safe And Root-Relative + +**Status: Pending** + +### Goal + +Fix the restore correctness gap so planning can never follow a leaf or ancestor +symlink and classify its target as an already matching regular file. + +### Required Work + +1. Open the selected local session root once for restore planning. If it does + not exist, create it with workspace directory modes before opening it. Pass + normalized session-relative names—not trusted absolute producer paths—to the + local classifier. Continue deriving display/canonical absolute paths only + through `artifacts.SessionPaths` and `joinWithinSessionRoot`. +2. Add a private app-owned target inspection routine that walks each existing + relative ancestor with `Root.Lstat` and `Root.OpenRoot`, without following a + symlink. Return a small semantic state: missing, regular opened file, + directory conflict, or non-regular conflict. A missing ancestor means the + target is missing; an ancestor symlink, leaf symlink, or other special entry + means non-regular conflict. Do not add inode snapshots, `SameFile` checks, + race hooks, or a new generic FileOps abstraction. +3. Refactor `classifyRestoreAction` to consume that semantic state. Hash only + the already opened regular file. Preserve committed-checksum comparison, + legacy remote-content comparison, audio identity rules, size comparison, + deterministic reasons/counts/order, and typed conflict categories. +4. Ensure `--force` changes only a content mismatch on a regular file into + `RestoreActionDownload`. Directory and non-regular conflicts, including + every symlink case, remain conflicts with force and non-force. Dry-run must + report the same classification without local mutation. +5. Apply the classifier to committed current artifacts, retained legacy current + state, and previous-cache actions. Do not remove legacy restore planning; + that is Workstream 3. +6. Add focused planner and command tests covering a matching leaf symlink, a + differing leaf symlink, an ancestor symlink, directory/special-file conflict, + missing ancestor, matching regular content, differing regular content with + and without force, and unchanged dry-run behavior. Assert classification and + safe outcomes rather than complete reason prose. +7. Update `docs/internal/command-restore.md`, `docs/operations.md`, and the + relevant restore troubleshooting guidance to make the implemented + root-relative symlink-conflict behavior explicit. + +### Exit Criteria + +- `go test ./internal/app -run 'Restore|restore'` and + `go test ./internal/artifacts ./internal/previouscache` succeed. +- Leaf and ancestor symlinks are non-regular conflicts even when their targets + have the expected bytes and even with `--force`. +- Only an opened regular target beneath the selected session root is hashed. +- Restore order, remote authority, legacy compatibility, and dry-run output + remain otherwise intact. +- `git diff --check` succeeds. + +## Stage 6 — Install Restore Results Through The Session Root + +**Status: Pending** + +### Goal + +Make execution preserve the planner's root and conflict decisions while keeping +complete validation and manifest-last recovery semantics. + +### Required Work + +1. Rework `executeRestorePlan` and `executeRestoreDownloadAction` around an + explicitly opened session `os.Root`. Normalize every action's relative path, + re-derive its canonical absolute path, and reject a plan whose recorded + absolute path disagrees. Keep the manifest action last. +2. Before writing, walk the destination parent under the root. Reject a + symlink/non-directory ancestor; create a missing component one at a time and + apply `WorkspaceDirectoryMode`. Immediately before install, reject a leaf + symlink, directory, or special file. Permit replacement only of an eligible + regular file when the plan selected download/force, or installation into an + absent leaf. +3. Create each temporary download as a sibling inside the validated parent. + Stream remote or verified in-memory content, close it, and validate all + applicable size, SHA-256, generation, non-empty, audio, and format contracts + before replacement. Install with workspace file mode and remove temp files + on all failures. +4. Preserve `verifyRestoredObject`'s committed identity revalidation immediately + before install. Preserve audio materialization/cache identity, but ensure the + final destination also passed root-relative parent/leaf validation. +5. For session and previous manifests, parse, validate, and rebase the complete + temporary content first, then serialize/install the validated manifest + atomically at its root-approved destination. Preserve current-session + identity checks and install the selected session manifest after all other + durable files. +6. Preserve the incomplete-restore marker, shared session lock, report status, + failure attribution, retry behavior, and prior successful installs. A failed + action must leave any prior canonical destination intact and keep the marker + blocking normal runner reuse. +7. Add execution-level regression tests for a parent or leaf becoming a + symlink between planning and execution, force refusal of non-regular targets, + validation failure preserving old bytes, temp cleanup, group modes, manifest- + last ordering, partial failure/retry, and successful regular replacement. + Use a small invocation-scoped seam only if needed to trigger the plan/execute + state change; do not add a package global race hook. +8. Update `docs/internal/command-restore.md`, `docs/internal/workspace.md`, + `docs/operations.md`, and `docs/troubleshooting.md` with the root-scoped + installation and recovery behavior. Avoid repeating path names owned by the + operations document. + +### Exit Criteria + +- `go test ./internal/app -run 'Restore|restore'`, + `go test ./internal/audio ./internal/manifest`, and representative restore- + then-run workflow tests succeed. +- Execution cannot install through an ancestor symlink or over any non-regular + leaf, including under `--force`. +- Failed transfer, identity, format, or manifest validation preserves the old + destination, removes temp state, and leaves recovery state accurate. +- Successful restore retains manifest-last ordering, workspace modes, and safe + retry. +- `git diff --check` succeeds. + +## Stage 7 — Replace Notarius Tree Copy With One Validated Rename + +**Status: Pending** + +### Goal + +Eliminate duplicate Notarius artifact trees and all custom atomic no-replace +directory-promotion machinery. + +### Required Work + +1. Keep `internal/adapters/notarius` responsible for v0.6 CLI construction, + receipt/index/management-document decoding, bounded JSON, bundle and lane + contract validation, and reference arguments. Do not move Notarius-specific + schema or path interpretation into FileOps. +2. In `internal/stage/extract.go`, establish one unique run-local Notarius output + root and the existing unique durable bundle destination. After the adapter + returns, require its bundle root to be a proper descendant of the requested + output root, reject an unsafe/empty result, verify the receipt identity, + reference snapshots, management documents, configured lanes, relative paths, + and staging checksums before installation. +3. Add the small shared workspace-tree mode normalizer decided by the roadmap. + Walk the validated bundle without following symlink entries; apply + `WorkspaceDirectoryMode` to directories and `WorkspaceFileMode` to regular + files. Reject symlinks and special entries because the Notarius bundle + contract—not general workspace policy—requires a regular tree. +4. Inspect the durable destination with `os.Lstat`; require it to be absent and + return a clear collision error otherwise. Ensure its parent has workspace + mode, then move the validated bundle with one `os.Rename`. Do not copy files, + create a second temp tree, sync either directory, or attempt an atomic + no-replace syscall. +5. Resolve promoted paths from the previously validated relative paths and + retain the existing post-install checksum/contract checks before returning + stage outputs. Preserve artifact IDs, contracts, provenance, selection, + metadata, manifest records, resume evidence, and all ten default D&D lanes. +6. Arrange scoped staging cleanup: derive cleanup authority from the canonical + run-local output root, not `adapterResult.BundleRoot`; on failure remove only + that verified invocation-owned staging root, and on success remove the empty + root after rename. Keep receipt and stderr diagnostics. A cleanup error should + be joined with the primary stage failure without obscuring it. +7. Delete `internal/fileops/directory.go`, `rename_noreplace_*.go`, + `sync_directory_*.go`, `ErrDirectorySyncUnsupported`, directory-promotion + hooks, custom tree-copy functions, and all mechanism/platform tests that now + have no contract. Remove remaining confined implementation files if no + retained helper uses them. +8. Rewrite extract tests around observable behavior: successful single rename, + all expected lanes immediately reusable, source tree absent, destination + collision, invalid/out-of-root adapter result, contract/checksum rejection + before install, mode normalization, failed-staging cleanup, receipt/log + retention, and no partial stage outputs. Do not assert a private rename + helper or syscall sequence. +9. Update `docs/integrations/notarius.md`, `docs/internal/stage-extract.md`, + `docs/internal/fileops.md`, `docs/internal/workspace.md`, + `docs/internal/overview.md`, `docs/operations.md`, and + `docs/troubleshooting.md`. Remove the Windows unsupported-promotion section, + duplicate pre-promotion-tree guidance, and atomic-no-replace claims. Describe + one validated run-local staging bundle moved to its immutable durable path. + +### Exit Criteria + +- `go test ./internal/adapters/notarius ./internal/stage ./internal/artifacts ./internal/fileops` succeeds. +- A successful extract retains exactly one complete durable bundle and no + pre-promotion copy, while all configured/default D&D outputs remain valid and + consumable. +- Invalid bundles and destination collisions install nothing; scoped staging is + cleaned and receipt/log diagnostics remain. +- Directory copy/promotion, no-replace platform code, directory syncing, + capability errors, and their mechanism tests are absent. +- Linux, macOS, and Windows builds compile without a platform-specific directory + promotion fallback. +- `git diff --check` succeeds. + +## Stage 8 — Finish The Filesystem Contract And Validate The Workstream + +**Status: Pending** + +### Goal + +Remove residual obsolete surfaces and claims, prove the reduced contract across +the repository, and close Workstream 2 without entering later refactors. + +### Required Work + +1. Inventory `internal/fileops` and every production caller. The final package + may contain only the settled atomic-file, bounded-read, workspace-mode/tree- + mode, and root-deletion helpers. Inline or delete a wrapper with one trivial + caller when direct standard-library code is clearer. Remove dead types, + sentinels, build-tag files, compatibility aliases, injection hooks, and + tests that protect retired behavior. +2. Search maintained source and canonical documentation for stale claims or + names involving durable/crash-safe replacement, directory sync, atomic + no-replace promotion, duplicate Notarius trees, universal confinement, + universal no-follow reads, and unsupported promotion platforms. Update only + current-behavior owners. Do not rewrite the audits, roadmaps, ADR history, + or historical release notes. +3. Review retained numeric limits and mode definitions for discoverability. + Keep each limit as a named package-level constant near its enforcing owner; + keep it deliberately above ordinary expected data where it is a disk/memory + safety guardrail; and ensure its error identifies the responsible adapter or + category and numeric limit. Do not add configuration knobs in this workstream. +4. Review API-key ingress end to end. Confirm secret values enter only through + existing environment/secret-file contracts, retain restrictive file handling, + and cannot appear in ordinary workspace files, subprocess arguments, + generated configuration, logs, manifests, reports, errors, examples, or test + diagnostics. +5. Consolidate redundant filesystem test fixtures within affected packages and + delete tests with no remaining plausible failure mode. Ensure one clear test + owner for atomic replacement, bounded reads, modes, cleanup root scope, + restore symlink classification/install, downloads, and Notarius rename. +6. Update `docs/internal/overview.md` and every affected focused internal link or + test inventory to the final file/package shape. Run the documentation link + and example checks after deleting files. +7. Mark Workstream 2 `Complete` in `docs/roadmap/refactoring.md`. Leave + `fileops.md`, the audit decision register, and the three source audits as the + durable decision record. Leave Workstream 3 as the next `Pending` workstream. ### 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: Completed** - -### 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. +- `scripts/build-release-assets.sh v0.0.0-test ABSOLUTE_EMPTY_TEMP_DIR` succeeds + for all six supported CGO-disabled release targets, and its host binary + version smoke check succeeds. Remove only that known temporary directory + afterward. +- `git diff --check` succeeds and the worktree contains no generated build + artifacts. +- Production no longer contains directory promotion/copy, directory sync, + hostile-race identity checks, or a general confined-filesystem facade. +- The retained filesystem code and canonical documentation match the accepted + `fileops.md` target state, Workstream 2 is `Complete`, and Workstreams 3–10 + remain unimplemented. ## 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. +None. The accepted feature roadmap, consolidated refactoring roadmap, audit +decision register, and decisions above are sufficient to implement all stages +without additional product or architecture choices.