672 lines
39 KiB
Markdown
672 lines
39 KiB
Markdown
# Workstream 2 Implementation Plan
|
||
|
||
## Purpose And Status
|
||
|
||
This document is the executable implementation plan for
|
||
[`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. 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:
|
||
|
||
- 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,
|
||
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
|
||
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 compatibility, manifest, publish,
|
||
cleanup-stage, or configuration code would also benefit from refactoring.
|
||
|
||
During every stage:
|
||
|
||
- 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 — Reduce Atomic Replacement, Modes, And Bounded Reads
|
||
|
||
**Status: Pending**
|
||
|
||
### Goal
|
||
|
||
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. 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 test ./...`, `go test -race ./...`, `go vet ./...`, and
|
||
`go build ./...` succeed.
|
||
- `go test ./internal/doccheck` and
|
||
`go test ./internal/config -run '^TestExamplesLoadAndValidate$'` succeed.
|
||
- `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 accepted feature roadmap, consolidated refactoring roadmap, audit
|
||
decision register, and decisions above are sufficient to implement all stages
|
||
without additional product or architecture choices.
|