Close out the completed audit
This commit is contained in:
@@ -25,7 +25,7 @@ polished transcripts and generated artifacts. Start with the
|
||||
| Adapters or external tool contracts | [Adapter Internals](internal/adapters.md) and [Integration Contracts](integrations/README.md) | The internal guide owns adapter composition and mechanics; integration documents own external formats and protocols. |
|
||||
| Manifests, artifacts, workspace paths, or publish behavior | [Manifest Internals](internal/manifest.md), [Artifact Internals](internal/artifacts.md), [Workspace Internals](internal/workspace.md), [Publish Internals](internal/stage-publish.md), and [Operations](operations.md) | These separate implementation state and resolution from operator-visible layout and lifecycle. |
|
||||
| Maintained configuration or input examples | [Configuration](config.md) and [Examples](../examples/README.md) | The reference owns field meanings; the examples directory owns complete copyable files. |
|
||||
| Proposed or unimplemented behavior | [Roadmap](roadmap/) | Future work belongs only in roadmap documentation until implemented. |
|
||||
| Proposed or unimplemented behavior | `docs/roadmap/` | Future work belongs only in roadmap documentation until implemented. |
|
||||
|
||||
For an existing subsystem, also inspect its focused tests and package-level
|
||||
contracts before changing behavior.
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -1,332 +0,0 @@
|
||||
# Codebase Audit Plan
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Purpose
|
||||
|
||||
This audit will evaluate Narratio for correctness, efficiency, maintainability,
|
||||
and test-suite value. It will identify defects and credible risks, duplicated or
|
||||
near-duplicated behavior, code that can be made smaller or more idiomatic, and
|
||||
complex code whose remaining invariants need focused explanation.
|
||||
|
||||
The audit is investigative. It should produce evidence-backed findings and a
|
||||
prioritized remediation backlog, not make opportunistic production changes as
|
||||
it proceeds. The [Audit Sequence](audit-sequence.md) assigns this scope to
|
||||
concrete execution stages.
|
||||
|
||||
## Authoritative Baseline
|
||||
|
||||
Review implemented behavior against its canonical owner rather than treating
|
||||
the current implementation or tests as the specification:
|
||||
|
||||
- [Architecture](../policy/architecture.md) for system boundaries, dependency
|
||||
direction, state and path ownership, safety properties, and pipeline
|
||||
invariants;
|
||||
- [Internal Overview](../internal/overview.md) and its focused internal
|
||||
documents for implemented ownership and mechanics;
|
||||
- [Testing Policy](../policy/testing.md) for risk-based sufficiency, durable
|
||||
boundaries, test-double guidance, and test lifecycle decisions;
|
||||
- the [CLI](../cli.md), [Configuration](../config.md),
|
||||
[Operations](../operations.md), and [integration contracts](../integrations/)
|
||||
for externally observable behavior; and
|
||||
- the [Documentation Policy](../policy/documentation.md) for canonical ownership
|
||||
and the distinction between current and proposed behavior.
|
||||
|
||||
Where code, tests, and documentation disagree, record the disagreement. Do not
|
||||
assume which one is wrong until the canonical contract and caller expectations
|
||||
have been traced.
|
||||
|
||||
## Audit Principles
|
||||
|
||||
1. Review correctness before cleanup. A shorter implementation is not an
|
||||
improvement if it weakens a state transition, safety check, or external
|
||||
contract.
|
||||
2. Trace behavior across boundaries. Narratio's most important properties often
|
||||
emerge from the interaction of application orchestration, stages, manifests,
|
||||
artifact resolution, filesystem operations, and adapters.
|
||||
3. Distinguish repeated syntax from repeated policy. Extract a helper only when
|
||||
the behavior has one stable owner and the shared abstraction makes that
|
||||
ownership clearer. Similar stage code may be intentionally explicit.
|
||||
4. Prefer narrow, idiomatic Go over generic frameworks. In particular, proposed
|
||||
refactors must preserve the explicit canonical stage sequence and must not
|
||||
turn Narratio into a workflow engine or a second configuration system for
|
||||
downstream tools.
|
||||
5. Optimize credible work. Flag repeated I/O, hashing, serialization, remote
|
||||
calls, subprocess work, allocation, or poor asymptotic behavior when the
|
||||
relevant path can matter. Require a benchmark or workload argument for
|
||||
performance changes whose benefit is not evident.
|
||||
6. Treat comments as explanations of intent. Recommend comments for invariants,
|
||||
ordering constraints, non-obvious failure policy, or security reasoning—not
|
||||
as narration of ordinary Go or a substitute for simplifying code.
|
||||
7. Judge tests as a suite. A test can be locally reasonable and still add no
|
||||
marginal protection, while a compact test can be inadequate for a
|
||||
consequential cross-component failure.
|
||||
|
||||
## Evidence And Finding Standard
|
||||
|
||||
Begin from a cleanly identified revision and record toolchain and platform
|
||||
assumptions. Use the code knowledge graph to find ownership, callers, callees,
|
||||
similarity candidates, high-complexity functions, and weakly protected
|
||||
boundaries. Confirm every candidate by reading the implementation, its focused
|
||||
tests, and the applicable contract. Text search and static analysis supplement
|
||||
the graph for literals, configuration, generated files, and patterns that are
|
||||
not modeled reliably.
|
||||
|
||||
Each finding should record:
|
||||
|
||||
- category: correctness defect, correctness risk, duplication, simplification,
|
||||
efficiency, architectural boundary, comment/clarity, or test-suite issue;
|
||||
- source locations and the affected contract or invariant;
|
||||
- concrete evidence and a realistic failure or maintenance scenario;
|
||||
- impact, likelihood, confidence, and estimated remediation scope separately;
|
||||
- the smallest plausible improvement and its intended owner;
|
||||
- tests that already protect the behavior, tests that should change or be
|
||||
added, and tests that may become redundant; and
|
||||
- dependencies on, or conflicts with, other findings.
|
||||
|
||||
Do not report a metric alone as a finding. Complexity, similarity, coverage,
|
||||
fan-in, file size, and test count are prioritization signals that require manual
|
||||
confirmation. Consolidate findings that share one root cause.
|
||||
|
||||
## Cross-Cutting Review Lenses
|
||||
|
||||
### Correctness And Pipeline Semantics
|
||||
|
||||
Construct an explicit lifecycle matrix for every stage outcome: first run,
|
||||
already-succeeded skip, self-skip, failure, interruption, forced replacement,
|
||||
non-resumable result, and successful rerun. Trace how each outcome changes the
|
||||
session manifest, invocation manifest, downstream stage state, artifacts,
|
||||
diagnostics, and cleanup eligibility.
|
||||
|
||||
Across the pipeline, verify:
|
||||
|
||||
- the registry exposes one deterministic canonical order;
|
||||
- each stage's declared inputs, outputs, configuration, adapters, and manifest
|
||||
effects agree with its implementation and focused documentation;
|
||||
- inputs are resolved through manifest and artifact contracts rather than
|
||||
incidental directory contents;
|
||||
- run-local outputs are fully validated before canonical materialization;
|
||||
- failure, cancellation, or process interruption cannot advertise partial work
|
||||
as successful;
|
||||
- force and changed outcomes invalidate exactly the intended succeeded
|
||||
downstream work;
|
||||
- repeated execution is idempotent where promised, and ordering is stable
|
||||
wherever maps, directory reads, remote listings, or dependency graphs are
|
||||
involved;
|
||||
- session, campaign, run, source, checksum, contract, and external provenance
|
||||
identities cannot be confused across runs; and
|
||||
- errors preserve useful causes and do not expose secrets or private content.
|
||||
|
||||
Use fault-oriented reasoning at durability boundaries: fail immediately before
|
||||
and after manifest saves, canonical renames, external process completion,
|
||||
uploads, current-manifest publication, the current-run commit marker, restore
|
||||
manifest installation, and cleanup. Determine which state is authoritative and
|
||||
whether the next invocation recovers safely.
|
||||
|
||||
### Duplication And Helper Ownership
|
||||
|
||||
Search for exact and semantic duplication in production and tests, including:
|
||||
|
||||
- repeated stage setup, input resolution, output validation, run-local
|
||||
materialization, metadata construction, and error adaptation;
|
||||
- repeated manifest create/load/save and session/run transition handling;
|
||||
- repeated adapter construction, timeout parsing, command execution, generated
|
||||
configuration, log handling, and output checks;
|
||||
- repeated source-ID, destination, remote-key, and path validation policy;
|
||||
- repeated sorting, deduplication, checksum, copy, and atomic-write mechanics;
|
||||
and
|
||||
- repeated test fixtures and assertions that encode the same policy at several
|
||||
layers.
|
||||
|
||||
For each candidate, decide whether it is coincidental similarity, a repeated
|
||||
mechanism, or duplicated policy. Recommend extraction only when the helper can
|
||||
have a clear package owner, a narrow contract, and callers that become easier
|
||||
to understand. Prefer an unexported local helper when sharing is package-local.
|
||||
Do not create a broad utility package, force unlike stage results into one data
|
||||
model, or move policy into storage/file-operation helpers.
|
||||
|
||||
Initial similarity and complexity signals should seed, but not predetermine,
|
||||
inspection of the single-stage command wrappers, session/run manifest
|
||||
persistence pairs, adapter constructors, Scriptorium operations, stage fakes,
|
||||
and common stage materialization paths.
|
||||
|
||||
### Simplification, Go Idioms, And Efficiency
|
||||
|
||||
Review long or branch-heavy functions for separable decisions, state
|
||||
transitions, or data transformations. Pay particular attention to orchestration,
|
||||
configuration validation, artifact dependency resolution, resume verification,
|
||||
restore/previous-cache planning, and analyze/publish selection logic. A useful
|
||||
refactor should reduce cognitive load while leaving the important ordering
|
||||
visible.
|
||||
|
||||
Check for:
|
||||
|
||||
- unnecessary nesting, defensive branches made unreachable by earlier
|
||||
validation, repeated normalization, and overly wide parameter lists;
|
||||
- interfaces defined for hypothetical extensibility rather than a demonstrated
|
||||
consumer boundary;
|
||||
- manual slice, map, string, error, and filesystem logic with a clearer standard
|
||||
library form;
|
||||
- incorrect or inconsistent `errors.Is`/`errors.As`, wrapping, context
|
||||
propagation, deferred cleanup, response-body closure, process waiting, and
|
||||
goroutine/channel ownership;
|
||||
- redundant filesystem scans, `stat`/checksum passes, whole-file buffering,
|
||||
copying, YAML/JSON round trips, sorting, remote listings, downloads, uploads,
|
||||
or adapter initialization;
|
||||
- linear searches nested in loops and repeated dependency or artifact lookup
|
||||
that should use an indexed map or a single planning pass;
|
||||
- unbounded concurrency, leaked work after cancellation, serialized independent
|
||||
work, and nondeterministic result collection; and
|
||||
- obsolete dependencies, portability assumptions, and platform-sensitive path
|
||||
or atomic-rename behavior.
|
||||
|
||||
Keep correctness and diagnosability ahead of micro-optimization. When a simpler
|
||||
algorithm changes performance characteristics, specify the representative
|
||||
input size and validation method.
|
||||
|
||||
### Comments And Local Explanation
|
||||
|
||||
Review high fan-in, high-complexity, security-sensitive, and commit-boundary
|
||||
code after likely simplifications have been identified. Add a comment
|
||||
recommendation when a maintainer needs to know why:
|
||||
|
||||
- state transitions or persistence operations occur in a specific order;
|
||||
- a stale record intentionally retains data while another transition clears it;
|
||||
- a path is checked more than once to resist traversal, symlink replacement, or
|
||||
time-of-check/time-of-use hazards;
|
||||
- an artifact is accepted only with particular manifest, checksum, contract, or
|
||||
provenance evidence;
|
||||
- a partial operation is intentionally not rolled back;
|
||||
- a remote pointer or local manifest must be installed last; or
|
||||
- concurrency, cancellation, compatibility, or downstream-tool behavior makes
|
||||
an apparently simpler approach unsafe.
|
||||
|
||||
Prefer a named helper, typed state, or smaller control flow when that removes the
|
||||
need for explanation. Check existing comments for stale claims as well as
|
||||
missing rationale.
|
||||
|
||||
### Test Suite Against The Canonical Policy
|
||||
|
||||
Build a risk-to-test matrix rather than auditing tests file by file in
|
||||
isolation. For each important behavior, identify its proper owner—parser,
|
||||
validator, domain package, adapter, orchestrator, CLI, integration, or end to
|
||||
end—and identify all tests that claim to protect it.
|
||||
|
||||
Evaluate:
|
||||
|
||||
- protection of data integrity, destructive operations, compatibility,
|
||||
security, concurrency, idempotency, recovery, and partial failure;
|
||||
- manifest transitions, force/invalidation, resume validation, atomic
|
||||
materialization, publish commit order, restore install order, and cleanup
|
||||
gates as assembled behaviors;
|
||||
- realistic HTTP, subprocess, filesystem, and object-store boundary behavior,
|
||||
including cancellation and malformed responses;
|
||||
- whether higher-level tests intentionally sample lower-level behavior or
|
||||
redundantly reproduce its full policy;
|
||||
- whether tests assert durable outcomes or private constants, exact error text,
|
||||
incidental paths, call choreography, or oversized snapshots;
|
||||
- whether real fast collaborators could replace elaborate doubles, and whether
|
||||
stateful fakes are realistic enough for the risk they protect;
|
||||
- fixture/helper duplication, oversized test cases, and setup that obscures the
|
||||
behavior under test without introducing a heavyweight test framework;
|
||||
- deterministic, offline, credential-free, order-independent execution and
|
||||
safe handling of environment and process-global state;
|
||||
- focused fuzz candidates in parsing, normalization, source IDs, remote/local
|
||||
path mapping, manifest decoding, and configuration boundaries; and
|
||||
- the presence and value of a small number of representative assembled
|
||||
workflows.
|
||||
|
||||
Use coverage only to locate unexpectedly weak consequential branches. Also
|
||||
inspect packages with extensive coverage for redundant tests and refactoring
|
||||
friction. For every proposed addition, deletion, or consolidation, state the
|
||||
realistic defect and marginal confidence involved.
|
||||
|
||||
The audit baseline should include the repository's canonical commands plus
|
||||
targeted diagnostic runs where supported:
|
||||
|
||||
```sh
|
||||
go test ./...
|
||||
go test -race ./...
|
||||
go vet ./...
|
||||
go build ./cmd/narratio
|
||||
```
|
||||
|
||||
Use focused repeated or shuffled runs to investigate state leakage and
|
||||
flakiness, and collect package/branch coverage for diagnosis. Review continuous
|
||||
integration to determine whether the appropriate offline validation is enforced;
|
||||
do not turn coverage percentage into a gate merely for this audit.
|
||||
|
||||
## Area-By-Area Inspection Map
|
||||
|
||||
| Area | Primary locations | What to inspect |
|
||||
| --- | --- | --- |
|
||||
| Process and application boundary | `cmd/narratio`, `internal/app` | Command dispatch, configuration selection, production composition, secret loading, lock lifetime, object-store initialization, context/error propagation, and separation of CLI reporting from orchestration policy. Review operator commands for consistent current-state authority and shared read-only mechanics. |
|
||||
| Stage registry and runner | `internal/stage/placeholders.go`, `internal/stage/stage.go`, `internal/app/planner.go`, `internal/app/runner.go`, `internal/app/run_stage.go` | Canonical order, action decisions, resume/force/self-skip/failure transitions, downstream invalidation, session/run manifest consistency, resource lifecycle, cleanup triggering, and opportunities to decompose the runner without hiding its state machine. |
|
||||
| Configuration | `internal/config` | Strict decoding, discovery and precedence, centralized defaults, normalization, templating, validation order, unknown fields, empty-value behavior, secret references, cross-field constraints, path confinement, deterministic errors, duplicated validator policy, and compatibility with maintained examples. |
|
||||
| Prepare and audio | `internal/stage/prepare.go`, `internal/audio`, `internal/previouscache` | Local/S3 exclusivity, cache and spool identity, partial downloads, checksum/reuse policy, previous-session required/optional planning, deterministic input records, clearing semantics, traversal safety, and avoiding repeated remote or filesystem work. |
|
||||
| Transcript stages | `internal/stage/transcribe.go`, `merge.go`, `polish.go`, `normalize.go`, `trim.go`, `render.go` | Contract parity across similar stages, bounded concurrency and cancellation, deterministic speaker/input ordering, run-local validation and canonical promotion, report/diagnostic classification, disabled behavior, and narrow opportunities for shared mechanics. |
|
||||
| Extraction | `internal/stage/extract.go`, `extract_resume.go`, `internal/adapters/notarius`, `internal/fileops/directory.go` | External receipt and lane validation, configuration fingerprint limits, immutable promotion, symlink/root replacement defenses, provenance and checksum checks, immediate and cross-invocation reuse, obsolete versus unsafe outcomes, failure residue, and whether dense verification logic can be clarified without weakening it. |
|
||||
| Analyze and artifact dependencies | `internal/stage/analyze.go`, `internal/artifacts`, `internal/artifactpolicy` | Source-family validation, runtime catalog state, enabled/selected/reused distinctions, topological ordering and cycle handling, required/optional inputs, local-only previous sources, deterministic metadata, repeated lookup/scanning, and ownership shared with config and publish. |
|
||||
| Publish and cleanup | `internal/stage/publish.go`, `internal/app/post_publish_cleanup.go`, `internal/app/cleanup_targets.go` | Prerequisite success, output selection, locks, required/optional behavior, exclusion rules, deterministic upload set, retry/idempotency implications, current-manifest then commit-marker ordering, metadata gates, and destructive path confinement. |
|
||||
| Manifest state | `internal/manifest` | Validation and backward compatibility, atomic persistence, timestamps, session/run identity, transition truth table, clearing versus retaining payload, create/load/save duplication, failure during dual-manifest updates, and whether state mutation has a single owner. |
|
||||
| Artifacts, paths, and policy | `internal/artifacts`, `internal/artifactpolicy`, `internal/pathsafe` | Canonical helper coverage, ad hoc reconstruction by callers, source-ID ownership, manifest-first resolution, extraction/current-state identity, destination normalization, stable ordering, typed missing-state errors, symlink/traversal defenses, and duplicate policy across config/stages/app. |
|
||||
| Restore | `internal/app/restore*.go`, `internal/previouscache`, `internal/audio` | Remote authority, confined mapping, deterministic plan actions, local conflict and force behavior, dry-run purity, temp-file installation, manifest-last ordering, partial failure/retry behavior, report accuracy, cache reuse, and shared current-state mechanics. |
|
||||
| File operations | `internal/fileops`, `internal/pathsafe`, local-store code in `internal/artifacts` | Atomic-write and promotion guarantees, permissions, close/sync/rename error handling, temp cleanup, same-filesystem assumptions, replacement policy, regular-file-only traversal, symlink and root-swap resistance, lock cleanup, and portability. |
|
||||
| External adapters and storage | `internal/adapters`, `internal/audio` | Transport isolation, shared subprocess mechanics versus adapter-specific policy, command/config duplication, quoting and working directories, timeouts/cancellation, stdout/stderr separation, HTTP body and retry behavior, S3 pagination/streaming/not-found mapping, credential independence, and external error adaptation. |
|
||||
| Shared models and diagnostics | `internal/artifactmodel`, `internal/contracts`, `internal/logging` | Serialization and validation invariants, unnecessary conversions, ownership of shared types, stable diagnostic structure, redaction, and whether small shared packages remain cohesive. |
|
||||
| Tests, examples, and automation | all `*_test.go`, `examples/`, `.woodpecker/` | Risk ownership, semantic duplication, fixture cost, policy-coupled assertions, realistic boundary tests, end-to-end sufficiency, default-suite isolation, example validation, diagnostic coverage, flakiness, runtime cost, and enforcement of canonical validation. |
|
||||
|
||||
## Narratio-Specific Cross-Boundary Scenarios
|
||||
|
||||
In addition to package-local review, trace these complete scenarios because a
|
||||
modular pipeline can look correct within every package while violating an
|
||||
end-to-end invariant:
|
||||
|
||||
1. A stage succeeds, its result becomes non-resumable, the rerun fails, and a
|
||||
later invocation decides what remains usable.
|
||||
2. An upstream forced or changed outcome interacts with already-succeeded,
|
||||
self-skipped, and disabled downstream stages.
|
||||
3. Extraction produces a valid immutable bundle, then configuration or
|
||||
transitive Notarius inputs change before analyze or publish.
|
||||
4. Previous-session state is published, restored or prepared into the local
|
||||
cache, and consumed by analyze without an unintended remote read.
|
||||
5. Publish fails at each upload boundary, especially between current manifest
|
||||
and current-run pointer, followed by status, restore, and retry.
|
||||
6. Restore encounters identical files, conflicting files, unsafe remote keys,
|
||||
cache hits, and a failure immediately before manifest installation.
|
||||
7. Automatic or manual cleanup is requested after skipped, failed, locked,
|
||||
partially uploaded, and fully committed publish outcomes.
|
||||
8. Cancellation reaches bounded transcription work, HTTP requests,
|
||||
subprocesses, object storage, and manifest reporting without leaks or false
|
||||
success.
|
||||
9. A configured artifact is disabled, unselected, reused, generated from
|
||||
another artifact, sourced from extraction, or sourced from a previous
|
||||
session, then filtered for publish.
|
||||
10. The same session is invoked concurrently, including lock contention and
|
||||
cleanup/release failures.
|
||||
|
||||
## Completion Criteria
|
||||
|
||||
The audit is complete when:
|
||||
|
||||
- every area in the inspection map has been reviewed against its canonical
|
||||
contracts and focused tests;
|
||||
- the stage lifecycle matrix and cross-boundary scenarios have explicit
|
||||
conclusions;
|
||||
- duplication candidates have been classified rather than merely counted;
|
||||
- simplification and performance recommendations explain their correctness
|
||||
constraints and expected benefit;
|
||||
- comment recommendations identify the non-obvious rationale to preserve;
|
||||
- the test suite has a risk-based sufficiency assessment, including gaps,
|
||||
redundancy, durability, execution properties, and automation;
|
||||
- findings are deduplicated, evidence-backed, and ranked by risk and dependency;
|
||||
and
|
||||
- unresolved questions and intentionally accepted risks are recorded rather
|
||||
than silently omitted.
|
||||
|
||||
## Execution
|
||||
|
||||
The [Audit Sequence](audit-sequence.md) is the canonical owner of execution
|
||||
order, stage boundaries, checkpoints, validation, and audit deliverables. This
|
||||
document remains the canonical owner of audit scope, review criteria, and the
|
||||
finding standard.
|
||||
@@ -1,734 +0,0 @@
|
||||
# Codebase Audit Sequence
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Purpose And Relationship To The Audit Plan
|
||||
|
||||
This document turns the [Codebase Audit Plan](audit-plan.md) into a bounded,
|
||||
execution-ready sequence. The plan owns scope, review criteria, and the finding
|
||||
standard. This document owns ordering, dependencies, working records,
|
||||
validation, and exit gates.
|
||||
|
||||
The sequence is for investigation only. Do not mix production refactors or bug
|
||||
fixes into the audit. A confirmed urgent defect may justify stopping to request
|
||||
a separate remediation change, but its fix is not part of this sequence.
|
||||
|
||||
## Audit Run Records
|
||||
|
||||
Create `docs/roadmap/audit-findings.md` when the audit begins. It is the single
|
||||
working ledger and final audit report. Initialize it with:
|
||||
|
||||
- the audited revision, branch/worktree state, Go version, platform, and audit
|
||||
date;
|
||||
- baseline command results and timings;
|
||||
- an area coverage ledger;
|
||||
- the stage lifecycle matrix;
|
||||
- the cross-boundary scenario matrix from the audit plan;
|
||||
- a risk-to-test matrix;
|
||||
- candidate and confirmed finding registers; and
|
||||
- unresolved questions, accepted risks, and final conclusions.
|
||||
|
||||
Track each execution stage in the coverage ledger with one of `not_started`,
|
||||
`in_progress`, `complete`, or `blocked`. For a completed stage, record:
|
||||
|
||||
- contracts, packages, files, and important symbols reviewed;
|
||||
- graph traces, commands, tests, or other evidence used;
|
||||
- finding and candidate IDs produced;
|
||||
- explicit no-finding conclusions for reviewed high-risk behavior; and
|
||||
- follow-up questions assigned to later stages.
|
||||
|
||||
Use stable finding IDs with these prefixes:
|
||||
|
||||
| Prefix | Category |
|
||||
| --- | --- |
|
||||
| `COR` | Confirmed correctness defect |
|
||||
| `RSK` | Correctness or operational risk |
|
||||
| `ARC` | Ownership or architectural-boundary issue |
|
||||
| `DUP` | Duplicated mechanism or policy |
|
||||
| `SIM` | Simplification or idiomatic-Go opportunity |
|
||||
| `EFF` | Efficiency or resource-use issue |
|
||||
| `COM` | Missing, misleading, or stale explanatory comment |
|
||||
| `TST` | Test-suite gap, redundancy, brittleness, or execution issue |
|
||||
|
||||
Candidate IDs remain candidates until manual inspection confirms the behavior,
|
||||
contract, realistic scenario, and affected callers. Rejected candidates remain
|
||||
in a short classification log so later stages do not reopen them without new
|
||||
evidence.
|
||||
|
||||
## Execution Rules
|
||||
|
||||
1. Pin the audit to the revision recorded in Stage 0. If the worktree or HEAD
|
||||
changes, record the change and rerun every affected stage; do not silently
|
||||
combine evidence from different implementations.
|
||||
2. Use codebase graph search and call/data-flow traces before broad source
|
||||
search. Read the exact implementation, focused tests, and canonical contract
|
||||
before confirming a finding.
|
||||
3. Record test-policy observations during every behavior pass. Stage 12 owns the
|
||||
suite-wide conclusion but must not rediscover the suite from scratch.
|
||||
4. Record cross-area observations as candidates for the stage that owns the
|
||||
conclusion. Avoid producing duplicate findings from several review passes.
|
||||
5. Treat baseline failures as evidence, not automatic blockers. Continue when
|
||||
read-only inspection remains sound, and state the limitation. Stop only when
|
||||
the repository cannot be identified, required sources are unavailable, or a
|
||||
failure makes later evidence unreliable.
|
||||
6. Do not exercise a suspected destructive, credentialed, paid, or live-service
|
||||
path merely to prove a defect. Use source reasoning, existing safe fakes, or
|
||||
a narrowly controlled offline reproduction.
|
||||
7. Escalate a credible active data-loss, secret-exposure, or unsafe-cleanup
|
||||
defect immediately. Preserve the evidence and do not wait for final
|
||||
synthesis before reporting it.
|
||||
8. A stage is complete only when its exit gate is met. A package test passing is
|
||||
evidence, not proof that the review is complete.
|
||||
|
||||
## Sequence Overview
|
||||
|
||||
| Stage | Focus | Depends on | Primary result |
|
||||
| --- | --- | --- | --- |
|
||||
| 0 | Pin revision and establish baseline | None | Reproducible audit record |
|
||||
| 1 | Contract, boundary, and lifecycle map | 0 | Review matrices and ownership map |
|
||||
| 2 | Runner and manifest state machine | 1 | Lifecycle and dual-ledger conclusions |
|
||||
| 3 | Paths, artifacts, and filesystem safety | 1-2 | State/path authority and mutation conclusions |
|
||||
| 4 | Publish, remote commit, and cleanup | 2-3 | Commit-boundary and destructive-operation conclusions |
|
||||
| 5 | Restore and remote/previous state | 2-4 | Restore authority and recovery conclusions |
|
||||
| 6 | Configuration and application composition | 1-5 | Validation and wiring conclusions |
|
||||
| 7 | External adapters and shared support | 3, 6 | Boundary, cancellation, and resource conclusions |
|
||||
| 8 | Prepare and transcript-processing stages | 2-3, 6-7 | Ordinary stage-contract conclusions |
|
||||
| 9 | Extraction vertical slice | 2-3, 6-7 | Promotion, provenance, and resume conclusions |
|
||||
| 10 | Analyze and artifact dependency slice | 3, 6, 8-9 | Dependency and source-resolution conclusions |
|
||||
| 11 | Cross-codebase duplication, simplicity, efficiency, and comments | 2-10 | Classified maintainability candidates |
|
||||
| 12 | Test-suite policy audit | 2-11 | Risk-based suite sufficiency assessment |
|
||||
| 13 | Synthesis and audit closeout | 0-12 | Final deduplicated audit report |
|
||||
|
||||
Stages are intentionally ordered. Later stages may resolve candidates raised by
|
||||
earlier ones, but they must not invalidate an earlier stage silently. Return to
|
||||
the owning stage, update its coverage record, and note the new evidence.
|
||||
|
||||
## Stage 0: Pin Revision And Establish Baseline
|
||||
|
||||
### Entry
|
||||
|
||||
- Repository root and `docs/development.md` are available.
|
||||
- The audit plan and canonical policy documents can be read.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Record `git rev-parse HEAD`, branch/detached state, `git status --short`,
|
||||
`go version`, `go env GOOS GOARCH`, and the current date.
|
||||
2. Confirm that the code knowledge graph represents the recorded repository and
|
||||
revision; refresh the index if it is missing or stale.
|
||||
3. Capture the package/file/test inventory, entry points, architecture
|
||||
boundaries, high fan-in symbols, complexity signals, and similarity signals.
|
||||
4. Run the default offline baseline and record wall time and failures:
|
||||
|
||||
```sh
|
||||
go test -count=1 ./...
|
||||
go test -race -count=1 ./...
|
||||
go vet ./...
|
||||
```
|
||||
|
||||
5. Build into an external temporary directory so validation does not add a
|
||||
workspace binary:
|
||||
|
||||
```sh
|
||||
audit_build_dir="$(mktemp -d)"
|
||||
go build -o "$audit_build_dir/narratio" ./cmd/narratio
|
||||
go test -coverprofile="$audit_build_dir/coverage.out" ./...
|
||||
```
|
||||
|
||||
6. Inventory the repository's CI/release validation, maintained examples, fuzz
|
||||
tests, golden data, opt-in tests, and generated-test update mechanisms.
|
||||
|
||||
### Output
|
||||
|
||||
- Baseline and inventory sections in `audit-findings.md`.
|
||||
- Initial coverage ledger containing Stages 0-13.
|
||||
- Unconfirmed metric-driven candidates, clearly labeled as such.
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Revision and environment are reproducible.
|
||||
- Every baseline command has a recorded result.
|
||||
- Graph freshness is known.
|
||||
- Any limitation that affects later stages has an owner and disposition.
|
||||
|
||||
## Stage 1: Build The Contract, Boundary, And Lifecycle Map
|
||||
|
||||
### Entry
|
||||
|
||||
- Stage 0 is complete.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Read the architecture, internal overview, testing policy, focused internal
|
||||
documents, and the relevant CLI/configuration/operations/integration
|
||||
contracts using the development guide's routing rules.
|
||||
2. Map each package and important interface to its owned policy. Mark every
|
||||
cross-package dependency that appears to reverse or blur the intended
|
||||
direction for later confirmation.
|
||||
3. Build a stage-contract matrix with canonical order, declared inputs,
|
||||
outputs, configuration, adapters, skip behavior, resume validation,
|
||||
materialization boundary, manifest effects, and downstream invalidation.
|
||||
4. Build the lifecycle matrix required by the audit plan: first run,
|
||||
already-succeeded skip, self-skip, failure, interruption, forced replacement,
|
||||
non-resumable result, and successful rerun.
|
||||
5. Assign each of the ten cross-boundary scenarios in the audit plan to its
|
||||
primary execution stage and list supporting packages/tests.
|
||||
6. Seed the risk-to-test matrix with the intended test owner for each
|
||||
architectural invariant. Do not judge sufficiency yet.
|
||||
|
||||
### Output
|
||||
|
||||
- Package ownership, stage-contract, lifecycle, scenario, and preliminary
|
||||
risk-to-test matrices.
|
||||
- `ARC` and `RSK` candidates for apparent disagreements, without deciding from
|
||||
documentation alone which artifact is wrong.
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Every area in the audit plan's inspection map has an assigned stage.
|
||||
- Every architectural invariant has an implementation owner and intended test
|
||||
owner.
|
||||
- Unknown or contradictory contracts are explicitly recorded.
|
||||
|
||||
## Stage 2: Audit The Runner And Manifest State Machine
|
||||
|
||||
### Entry
|
||||
|
||||
- Stage 1 matrices are complete.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Trace the entry paths into full-run and single-stage execution through
|
||||
`internal/app/planner.go`, `runner.go`, `run_stage.go`, and related helpers.
|
||||
2. Inspect `internal/manifest` models, validation, session/run creation,
|
||||
loading, normalization, atomic saves, and all transition methods.
|
||||
3. Walk every lifecycle-matrix cell through both manifests. Verify clearing
|
||||
versus retention of outputs, diagnostics, generated configuration, metadata,
|
||||
errors, actions, timestamps, and downstream state.
|
||||
4. Reason about failures before and after each session-manifest and run-manifest
|
||||
save. Determine which disagreement states are possible and how a later
|
||||
invocation interprets them.
|
||||
5. Review force, changed-result, self-skip, failed-result, and non-resumable
|
||||
invalidation separately. Confirm behavior at the first and last canonical
|
||||
stage.
|
||||
6. Review session lock acquisition/release and concurrent invocation behavior,
|
||||
while leaving path implementation details to Stage 3.
|
||||
7. Classify the runner's complexity and repeated session/run persistence paths:
|
||||
state-machine clarity, justified explicitness, candidate local helpers, and
|
||||
comments that preserve ordering rationale.
|
||||
8. Review focused app/manifest tests against the matrix and add observations to
|
||||
the risk-to-test ledger.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test -count=1 ./internal/app ./internal/manifest
|
||||
go test -race -count=1 ./internal/app ./internal/manifest
|
||||
```
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Every lifecycle cell has a source-backed conclusion for both manifests.
|
||||
- Cross-boundary scenarios 1, 2, and the lock portion of 10 are resolved or
|
||||
carry explicit questions.
|
||||
- All runner/manifest candidates are confirmed, rejected, or assigned to a
|
||||
named later stage.
|
||||
|
||||
## Stage 3: Audit Paths, Artifacts, And Filesystem Safety
|
||||
|
||||
### Entry
|
||||
|
||||
- Stages 1-2 are complete.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Review `internal/artifacts`, `internal/artifactpolicy`, `internal/pathsafe`,
|
||||
`internal/fileops`, and local-store filesystem code.
|
||||
2. Inventory canonical path and key helpers, then search callers for ad hoc
|
||||
reconstruction, double normalization, mixed slash/filesystem semantics, or
|
||||
policy implemented outside its owner.
|
||||
3. Trace built-in, configured, extraction, previous-session, and current-state
|
||||
artifact resolution. Verify identity, checksum, contract, provenance,
|
||||
deterministic ordering, and typed missing-state behavior.
|
||||
4. Review atomic file writes, copies, directory promotion, temp cleanup,
|
||||
permission preservation, close/sync/rename errors, existing-destination
|
||||
behavior, same-filesystem assumptions, and platform sensitivity.
|
||||
5. Walk traversal, absolute path, broad root, symlink component, inspected-root
|
||||
replacement, non-regular file, and time-of-check/time-of-use scenarios.
|
||||
6. Confirm that low-level file/storage helpers receive explicit destinations
|
||||
and do not infer stage, campaign, session, run, or publish policy.
|
||||
7. Inspect lock-file implementation and cleanup errors to finish scenario 10.
|
||||
8. Record focused test ownership and gaps without duplicating Stage 2's state
|
||||
conclusions.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test -count=1 ./internal/artifacts ./internal/artifactpolicy ./internal/pathsafe ./internal/fileops
|
||||
go test -race -count=1 ./internal/artifacts ./internal/fileops
|
||||
```
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Every canonical path/key family has one identified owner.
|
||||
- Every material filesystem mutation has documented confinement and atomicity
|
||||
conclusions.
|
||||
- Scenario 10 is resolved.
|
||||
- Safety checks that appear repetitive are classified before any simplification
|
||||
recommendation is made.
|
||||
|
||||
## Stage 4: Audit Publish, Remote Commit, And Cleanup
|
||||
|
||||
### Entry
|
||||
|
||||
- Stages 2-3 are complete.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Trace publish from stage selection through object-store calls, manifest
|
||||
metadata, commit-marker publication, run completion, and post-publish
|
||||
cleanup.
|
||||
2. Verify prerequisite stage-state checks, selected/configured/extraction
|
||||
output resolution, required versus optional outputs, static and remote
|
||||
locks, run-file exclusions, previous-cache inclusion, and deterministic
|
||||
upload order.
|
||||
3. Enumerate failures before and after every upload. Prove that
|
||||
`current/run_id.txt` is written last and is the only remote-current commit
|
||||
point.
|
||||
4. Review retry/idempotency behavior, existing remote objects, partial uploads,
|
||||
pointer/manifest disagreement, and status/restore interpretation after each
|
||||
partial outcome.
|
||||
5. Trace automatic and manual cleanup gates. Confirm publish execution,
|
||||
`uploaded`, `current_pointer_written`, explicit policy, and confined targets
|
||||
are all required at the correct boundary.
|
||||
6. Confirm that `--force` cannot override publish locks or cleanup safety.
|
||||
7. Review duplication between publish planning, artifact destination policy,
|
||||
operator views, and cleanup metadata only after ownership is established.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test -count=1 ./internal/stage ./internal/app ./internal/artifacts ./internal/adapters/storage
|
||||
```
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Cross-boundary scenarios 5 and 7 are resolved for every relevant failure
|
||||
boundary.
|
||||
- Remote-current authority and local-cleanup eligibility have explicit truth
|
||||
tables.
|
||||
- Publish findings distinguish stage policy from storage mechanics.
|
||||
|
||||
## Stage 5: Audit Restore And Remote/Previous State
|
||||
|
||||
### Entry
|
||||
|
||||
- Stages 2-4 are complete.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Trace restore discovery, planning, execution, reporting, audio
|
||||
materialization, and previous-cache planning through `internal/app`,
|
||||
`internal/artifacts`, `internal/previouscache`, `internal/audio`, and storage.
|
||||
2. Confirm remote pointer/manifest identity and campaign/session/run authority,
|
||||
including missing and inconsistent current state.
|
||||
3. Verify remote-to-local confinement, deterministic action ordering,
|
||||
`download`/`skip_same`/`conflict` decisions, force semantics, and dry-run
|
||||
purity.
|
||||
4. Walk failures during download, checksum or manifest validation, atomic
|
||||
install, report persistence, and the manifest-last boundary. Record the
|
||||
intentional lack of rollback and retry consequences.
|
||||
5. Review audio spool/cache identity, cache-hit verification, partial download
|
||||
behavior, and duplicate remote/filesystem work.
|
||||
6. Review previous-session requirement planning, required/optional behavior,
|
||||
identity checks, published-path fallback, and deterministic local mapping.
|
||||
7. Confirm which mechanics are shared with status/validate/operator commands
|
||||
and which caller-specific missing-state policies must remain separate.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test -count=1 ./internal/app ./internal/previouscache ./internal/audio ./internal/artifacts ./internal/adapters/storage
|
||||
```
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Cross-boundary scenarios 4 and 6 are resolved through retry/recovery.
|
||||
- Restore authority, manifest-last installation, and partial-write behavior are
|
||||
explicit.
|
||||
- Previous-cache conclusions are ready for the prepare and analyze passes.
|
||||
|
||||
## Stage 6: Audit Configuration And Application Composition
|
||||
|
||||
### Entry
|
||||
|
||||
- Stage 1 is complete and Stages 2-5 have identified the policies that
|
||||
configuration and composition must supply.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Review `internal/config`, `cmd/narratio`, application command dispatch,
|
||||
configuration selection, secret-file environment loading, and production
|
||||
collaborator construction.
|
||||
2. Trace discovery, precedence, strict YAML decoding, defaults, empty values,
|
||||
normalization, session templating, and validation order across pipeline,
|
||||
campaign, and session configuration.
|
||||
3. Verify cross-field constraints for stage enablement, paths, timeouts,
|
||||
concurrency, artifacts, Notarius, Scriptorium, publish, storage, cleanup,
|
||||
audio, and previous-session behavior.
|
||||
4. Compare validation logic with maintained examples and the public
|
||||
configuration contract. Record contract drift rather than silently choosing
|
||||
code or docs.
|
||||
5. Check that filesystem secrets are loaded before the boundary that consumes
|
||||
them and are excluded from logs, manifests, reports, generated files, and
|
||||
errors.
|
||||
6. Review conditional construction of expensive/external collaborators and
|
||||
cleanup of anything with a lifecycle. Confirm test injection cannot create a
|
||||
behavior different from production composition.
|
||||
7. Classify repeated validators, path checks, timeout parsing, constructor
|
||||
wrappers, and single-stage command wrappers by policy owner.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test -count=1 ./internal/config ./internal/app ./cmd/narratio
|
||||
go vet ./...
|
||||
```
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Every operator-visible field used by audited behavior has a traced default,
|
||||
normalization, validation, and consumer.
|
||||
- Composition conclusions cover enabled and disabled stages without requiring
|
||||
live services or credentials.
|
||||
- Maintained examples have an explicit validity conclusion.
|
||||
|
||||
## Stage 7: Audit External Adapters And Shared Support
|
||||
|
||||
### Entry
|
||||
|
||||
- Stages 3 and 6 are complete.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Review `internal/adapters`, `internal/audio`, `internal/logging`,
|
||||
`internal/contracts`, and `internal/artifactmodel` at their public package
|
||||
boundaries.
|
||||
2. For each HTTP, subprocess, notification, and object-storage adapter, compare
|
||||
implementation with its integration contract and trace all production
|
||||
callers.
|
||||
3. Verify context cancellation, timeout ownership, process termination and
|
||||
waiting, goroutine/channel closure, HTTP response-body closure, retries,
|
||||
malformed responses, streaming, pagination, not-found mapping, and local
|
||||
file cleanup.
|
||||
4. Confirm command argument construction, working directory, environment,
|
||||
generated configuration, stdout/stderr separation, output validation, and
|
||||
external error adaptation stay inside the owning adapter.
|
||||
5. Compare subprocess implementations to the shared subprocess package. Classify
|
||||
repeated constructor/config/log/output mechanics separately from
|
||||
adapter-specific protocol policy.
|
||||
6. Review fakes for realistic state and concurrency behavior, but defer their
|
||||
suite-wide value judgment to Stage 12.
|
||||
7. Check shared models for avoidable conversions, stable serialization,
|
||||
validation ownership, and redaction-sensitive diagnostic fields.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test -count=1 ./internal/adapters/... ./internal/audio ./internal/logging ./internal/contracts ./internal/artifactmodel
|
||||
go test -race -count=1 ./internal/adapters/... ./internal/audio
|
||||
```
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Every external resource has an explicit acquisition, cancellation, and
|
||||
release conclusion.
|
||||
- Transport types and protocol policy have not leaked into stages.
|
||||
- Adapter duplication candidates identify the correct shared or specific
|
||||
owner.
|
||||
|
||||
## Stage 8: Audit Prepare And Transcript-Processing Stages
|
||||
|
||||
### Entry
|
||||
|
||||
- Stages 2-3 and 6-7 are complete.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Review `prepare`, `transcribe`, `merge`, `polish`, `normalize`, `trim`, and
|
||||
`render` as vertical slices from resolved configuration and manifest input
|
||||
through adapter call, run-local output, validation, canonical
|
||||
materialization, and recorded result.
|
||||
2. Verify each implementation against the Stage 1 contract matrix and focused
|
||||
internal document. Record any undeclared input, output, diagnostic, config,
|
||||
adapter, or skip/failure behavior.
|
||||
3. For prepare, confirm local/S3 exclusivity, stable input copying,
|
||||
previous-cache clearing/hydration, and deterministic manifest input records.
|
||||
4. For transcribe, confirm unique speaker identities, bounded runtime
|
||||
concurrency, cancellation, deterministic result ordering, adapter-returned
|
||||
path identity, and partial failure behavior.
|
||||
5. For transformation/render stages, confirm manifest-first resolution,
|
||||
run-local paths, schema/report validation, disabled/default behavior,
|
||||
canonical promotion, and diagnostic-versus-artifact classification.
|
||||
6. Compare similar stage implementations for shared mechanisms only after
|
||||
listing meaningful differences. Avoid a generic stage framework.
|
||||
7. Add stage-focused test ownership, gaps, and redundancy candidates to the
|
||||
risk-to-test matrix.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test -count=1 ./internal/stage ./internal/audio ./internal/previouscache ./internal/adapters/whisperx ./internal/adapters/seriatim ./internal/adapters/audita ./internal/adapters/scriptorium
|
||||
go test -race -count=1 ./internal/stage ./internal/audio
|
||||
```
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Every reviewed stage has a completed contract-matrix row.
|
||||
- Cross-boundary scenario 8 is resolved for transcription and subprocess-backed
|
||||
transformation stages.
|
||||
- Similarity candidates are classified as intentional explicitness, local
|
||||
helper candidates, or shared-owner findings.
|
||||
|
||||
## Stage 9: Audit The Extraction Vertical Slice
|
||||
|
||||
### Entry
|
||||
|
||||
- Stages 2-3 and 6-7 are complete.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Trace extraction from configuration validation and composition through
|
||||
transcript resolution, invocation fingerprint, Notarius execution, receipt
|
||||
and lane validation, directory promotion, manifest recording, catalog
|
||||
hydration, resume validation, analyze, and publish consumers.
|
||||
2. Verify run-local isolation, regular-file and confined-index requirements,
|
||||
required-lane policy, contract/provenance construction, checksum timing,
|
||||
immutable destination identity, and no-replacement promotion.
|
||||
3. Enumerate failures before and after subprocess completion, receipt parsing,
|
||||
payload inspection, promotion, and manifest persistence. Determine what
|
||||
remains diagnostic, durable, advertised, and reusable.
|
||||
4. Walk every resume validation branch. Distinguish obsolete/missing outcomes
|
||||
that trigger rerun from unsafe conditions that must stop execution.
|
||||
5. Evaluate the fingerprint's intentionally observable and unobservable inputs
|
||||
against documentation and force guidance.
|
||||
6. Review the dense validation code for named sub-decisions and comments while
|
||||
preserving the visible security proof and check ordering.
|
||||
7. Confirm focused tests cover immediate reuse, cross-invocation reuse,
|
||||
configuration change, payload tampering, provenance mismatch, symlinks/root
|
||||
replacement, failure residue, and downstream invalidation at the correct
|
||||
layers.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test -count=1 ./internal/stage ./internal/artifacts ./internal/fileops ./internal/adapters/notarius ./internal/app
|
||||
```
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Cross-boundary scenario 3 is resolved, including transitive-input limits.
|
||||
- Promotion, advertisement, and resume each have a distinct authority and
|
||||
failure conclusion.
|
||||
- Every proposed simplification states which security or compatibility checks
|
||||
it preserves.
|
||||
|
||||
## Stage 10: Audit Analyze And Artifact Dependencies
|
||||
|
||||
### Entry
|
||||
|
||||
- Stages 3, 6, 8, and 9 are complete.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Trace all analyze source families from configuration validation through
|
||||
runtime catalog registration, availability, resolution, Scriptorium
|
||||
execution/reuse, materialization, metadata, and publish selection.
|
||||
2. Verify enabled, selected, executable, reused, generated, and unavailable
|
||||
states are distinct and deterministic.
|
||||
3. Review configured-artifact dependency validation and runtime topological
|
||||
ordering for cycles, missing dependencies, stable ordering, and consistency
|
||||
between configuration and execution.
|
||||
4. Confirm required/optional behavior and guidance for built-in transcripts,
|
||||
prepared stable inputs, configured artifacts, extraction sources, and
|
||||
previous-session sources.
|
||||
5. Prove previous-session resolution is local-only during analyze and that
|
||||
disabled artifacts are reused only under the documented conditions.
|
||||
6. Inspect repeated resolution branches, parameter width, nested lookup, and
|
||||
ordering work for a smaller representation or indexed plan without merging
|
||||
distinct source policies.
|
||||
7. Review tests for each state transition and source family at the narrowest
|
||||
stable owner, noting semantic duplication across config, artifacts, stage,
|
||||
publish, and assembled runner tests.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test -count=1 ./internal/stage ./internal/artifacts ./internal/artifactpolicy ./internal/config ./internal/adapters/scriptorium ./internal/app
|
||||
```
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Cross-boundary scenario 9 is resolved for every source family and selection
|
||||
state.
|
||||
- Dependency ordering and source availability have explicit determinism and
|
||||
complexity conclusions.
|
||||
- Config, artifact-policy, catalog, stage, and publish ownership is unambiguous
|
||||
or represented by an `ARC` finding.
|
||||
|
||||
## Stage 11: Audit Duplication, Simplicity, Efficiency, And Comments
|
||||
|
||||
### Entry
|
||||
|
||||
- Behavior stages 2-10 are complete, so structural candidates can be judged
|
||||
against known contracts.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Rerun graph similarity, complexity, fan-in/fan-out, call-path, loop-depth,
|
||||
scan-in-loop, and change-coupling analyses on production code. Add targeted
|
||||
text/static searches for patterns the graph cannot represent.
|
||||
2. Revisit all `DUP`, `SIM`, `EFF`, and `COM` candidates collected earlier.
|
||||
Search for additional occurrences and trace all callers before assigning an
|
||||
owner.
|
||||
3. For duplication, classify coincidental syntax, shared mechanism, duplicated
|
||||
policy, or deliberately explicit security/state logic. Propose only the
|
||||
narrowest helper that improves ownership and comprehension.
|
||||
4. For complexity, sketch the smaller control flow or data model and verify it
|
||||
leaves state transitions, validation order, and commit boundaries visible.
|
||||
5. For efficiency, state the input scale or call frequency, current and proposed
|
||||
complexity/I/O behavior, expected benefit, and benchmark or measurement
|
||||
needed. Reject micro-optimizations without a credible workload.
|
||||
6. Review standard-library usage, errors, slices/maps, allocations, copying,
|
||||
sorting, serialization, filesystem passes, adapter initialization, remote
|
||||
calls, goroutines/channels, and interface breadth across the complete codebase.
|
||||
7. Review comments only after simplification decisions. Recommend why-comments
|
||||
for remaining invariants, compatibility limits, safety checks, partial
|
||||
failure, and ordering; flag comments that restate code or no longer match it.
|
||||
8. Check dependencies and platform assumptions for clear correctness,
|
||||
portability, complexity, or maintenance consequences.
|
||||
|
||||
### Validation
|
||||
|
||||
- Run focused package tests for any behavior used to disprove or confirm a
|
||||
candidate.
|
||||
- Run existing benchmarks where relevant. Propose a benchmark rather than
|
||||
inventing performance claims when representative measurement is absent.
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Every structural candidate is confirmed, rejected with a reason, or merged
|
||||
into a stronger root-cause finding.
|
||||
- No helper recommendation creates a generic workflow abstraction or moves
|
||||
policy into a low-level utility.
|
||||
- Every efficiency finding has a credible workload and validation method.
|
||||
- Every comment finding states the non-obvious rationale that should be
|
||||
preserved.
|
||||
|
||||
## Stage 12: Audit The Test Suite Against Policy
|
||||
|
||||
### Entry
|
||||
|
||||
- Stages 2-11 have populated the risk-to-test matrix and test observations.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Complete the risk-to-test matrix. For every consequential invariant, list
|
||||
the current tests, proper owner, protected defect, missing failure modes, and
|
||||
overlap with other layers.
|
||||
2. Review tests by behavior cluster rather than filename: parsing/validation,
|
||||
domain/state, filesystem, adapters, orchestration, CLI, integration, and
|
||||
representative assembled workflows.
|
||||
3. Classify gaps for data integrity, destructive operations, compatibility,
|
||||
security, concurrency, idempotency, recovery, cancellation, and partial
|
||||
success. Confirm the gap is not credibly protected elsewhere.
|
||||
4. Classify redundancy and brittleness: private constants/defaults, exact error
|
||||
wording, incidental formatting/paths, mock choreography, oversized
|
||||
snapshots, helper-level duplication, and the same policy repeated across
|
||||
layers.
|
||||
5. Review doubles using the policy order: real deterministic collaborator,
|
||||
stateful fake, stub, then mock when interaction is contractual. Check that
|
||||
fakes model the failure and state semantics used by the tests.
|
||||
6. Inspect test helpers and large test functions for simplification and
|
||||
meaningful table-driven boundaries without creating a fixture framework
|
||||
whose maintenance cost exceeds its value.
|
||||
7. Review determinism and isolation: credentials, network access, paid APIs,
|
||||
environment, working directory, clocks, randomness, ports, temp paths,
|
||||
process-global state, ordering, cleanup, and parallel execution.
|
||||
8. Use coverage to investigate consequential weak branches, not as a score.
|
||||
Review heavily covered behavior for marginal-value duplication as well.
|
||||
9. Identify focused fuzz opportunities for parsers, YAML/JSON normalization,
|
||||
source IDs, confined paths, remote/local mapping, and manifest decoding.
|
||||
10. Compare local requirements with `.woodpecker/` and other automation. Record
|
||||
missing enforcement as a risk/cost decision, not an assumption that every
|
||||
diagnostic command belongs in CI.
|
||||
11. Investigate order dependence and flakiness with bounded runs, recording
|
||||
runtime and any reproducible seed:
|
||||
|
||||
```sh
|
||||
go test -shuffle=on -count=3 ./...
|
||||
go test -race -shuffle=on -count=1 ./...
|
||||
```
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Every important risk has a sufficiency conclusion and one intended test
|
||||
owner.
|
||||
- Every proposed test addition names the realistic defect and marginal value.
|
||||
- Every deletion/consolidation names the stronger remaining protection.
|
||||
- Default-suite determinism, offline behavior, runtime, flakiness, and CI
|
||||
enforcement have explicit conclusions.
|
||||
|
||||
## Stage 13: Synthesize And Close The Audit
|
||||
|
||||
### Entry
|
||||
|
||||
- Stages 0-12 meet their exit gates or have explicitly accepted limitations.
|
||||
|
||||
### Execute
|
||||
|
||||
1. Reconcile candidates and findings across stages. Merge shared root causes and
|
||||
remove repeated symptoms while retaining all affected locations and
|
||||
contracts.
|
||||
2. Recheck every confirmed finding against current source, callers, tests, and
|
||||
canonical documentation. Downgrade or reject anything supported only by a
|
||||
metric or hypothetical preference.
|
||||
3. Rank impact, likelihood, confidence, and remediation scope separately. Order
|
||||
the recommended backlog by dependency: correctness/data safety first,
|
||||
architectural ownership next, then simplification/duplication, tests,
|
||||
efficiency, and comments where they remain necessary.
|
||||
4. Record positive conclusions for high-risk areas where the current design and
|
||||
tests are sufficient. The report should not imply that only defective areas
|
||||
were reviewed.
|
||||
5. Reconcile the area coverage ledger, lifecycle matrix, cross-boundary scenario
|
||||
matrix, and risk-to-test matrix with the audit plan's completion criteria.
|
||||
6. Record any accepted risks, ambiguous contracts, environmental limitations,
|
||||
and deferred investigations with an explicit rationale and owner.
|
||||
7. Check whether HEAD or the worktree changed since Stage 0. Rerun affected
|
||||
stages or clearly pin the report to the original revision.
|
||||
8. Validate the report and roadmap document links and run `git diff --check`.
|
||||
If implementation changed during the audit, rerun the full Stage 0 validation
|
||||
baseline against the final audited revision.
|
||||
|
||||
### Final Deliverable
|
||||
|
||||
`docs/roadmap/audit-findings.md` must contain:
|
||||
|
||||
- an executive assessment without unsupported quality scores;
|
||||
- the audited revision and validation baseline;
|
||||
- coverage and scenario completion summaries;
|
||||
- confirmed findings ordered by dependency and risk;
|
||||
- rejected candidate themes where their recurrence would otherwise waste work;
|
||||
- the test-suite sufficiency assessment;
|
||||
- positive conclusions and accepted risks; and
|
||||
- a recommended remediation order, without implementing the remediation.
|
||||
|
||||
### Exit Gate
|
||||
|
||||
- Every completion criterion in the audit plan is satisfied or explicitly
|
||||
marked limited with rationale.
|
||||
- Every finding is evidence-backed, deduplicated, actionable, and assigned a
|
||||
stable ID.
|
||||
- No production change is included in the audit output.
|
||||
- The report is sufficient to prepare a separate remediation sequence without
|
||||
repeating discovery.
|
||||
@@ -1,104 +0,0 @@
|
||||
# Implementation Plan Summary
|
||||
|
||||
This is a concise, plain-language summary of
|
||||
[`implementation.md`](implementation.md). It describes the intended real-world
|
||||
outcome of each stage without reproducing its implementation details.
|
||||
|
||||
1. **Stage 1 — Collaborative workspace permissions:** Treat ordinary Narratio and
|
||||
Notarius data as shareable and make managed workspaces group-writable, while
|
||||
retaining private handling for API keys.
|
||||
2. **Stage 2 — Safe identifiers:** Reject campaign, session, run, artifact, and
|
||||
source identifiers that could escape their intended filesystem namespace, and
|
||||
use fuzz tests to cover platform-specific path tricks.
|
||||
3. **Stage 3 — Durable file replacement:** Consolidate duplicated atomic-write
|
||||
functions into one shared mechanism that fully persists a replacement before
|
||||
reporting success.
|
||||
4. **Stage 4 — Confined writes and downloads:** Rewrite destination mutations so
|
||||
symlinks or concurrent directory replacement cannot redirect writes,
|
||||
promotions, or downloads outside the intended root.
|
||||
5. **Stage 5 — Safe deletion and crash-recoverable locks:** Confine recursive
|
||||
cleanup to its authorized root and replace stale lock-file existence checks
|
||||
with operating-system locks released automatically after process death.
|
||||
6. **Stage 6 — Protected API-key reads:** Read API keys only from private,
|
||||
bounded, regular files without following symlinks or exposing key material in
|
||||
errors.
|
||||
7. **Stage 7 — Bounded external results:** Prevent external adapters from causing
|
||||
memory or disk exhaustion by validating regular result files and enforcing
|
||||
generous, clearly reported per-adapter size limits.
|
||||
8. **Stage 8 — Complete subprocess termination:** Ensure cancellation, timeout,
|
||||
or a safety-limit failure terminates and reaps an external command's entire
|
||||
process tree rather than only its parent process.
|
||||
9. **Stage 9 — Safe subprocess diagnostics:** Redact known credentials from
|
||||
stdout/stderr, cap persisted diagnostics, and terminate runaway producers when
|
||||
those caps are reached.
|
||||
10. **Stage 10 — Safe publish inputs:** Ensure publishing reads and uploads only
|
||||
verified regular files declared within the selected run, even during
|
||||
filesystem races.
|
||||
11. **Stage 11 — Consistent run identity:** Resolve one campaign/session/run
|
||||
identity for an invocation, reject conflicting authorities, and prevent stale
|
||||
identity fields from leaking into a later run.
|
||||
12. **Stage 12 — Reliable failure recording:** Consolidate terminal-failure
|
||||
persistence so handled errors reliably update authoritative session state and
|
||||
preserve any secondary persistence failures.
|
||||
13. **Stage 13 — Immutable remote-state model:** Define a versioned immutable
|
||||
remote snapshot selected by a small pointer, while isolating old-format read
|
||||
compatibility so it can be removed after migration.
|
||||
14. **Stage 14 — Transactional publication:** Upload and verify a complete
|
||||
immutable snapshot before one final pointer change makes it current, using an
|
||||
exact source-to-destination mapping instead of basename guesses.
|
||||
15. **Stage 15 — Safe remote locking and pagination:** Use provider-enforced
|
||||
conditional writes so publishers cannot overwrite another owner's lock, and
|
||||
fail instead of looping when object-store pagination stops making progress.
|
||||
16. **Stage 16 — Retryable cleanup:** Persist post-publication cleanup as a
|
||||
durable obligation so interrupted or failed deletion is retried and never
|
||||
mistaken for completed cleanup.
|
||||
17. **Stage 17 — Snapshot-consistent restore:** Make restore and status use one
|
||||
selected immutable snapshot throughout the operation, and prevent `--force`
|
||||
from overwriting unsafe directory or non-file conflicts.
|
||||
18. **Stage 18 — Race-safe, portable restore:** Serialize restore against runner
|
||||
reuse, leave durable evidence of incomplete restores, and replace unsafe
|
||||
producer-machine absolute paths with validated local references.
|
||||
19. **Stage 19 — Correct audio-cache reuse:** Reuse downloaded audio only when its
|
||||
local bytes and recorded metadata match the selected remote object version.
|
||||
20. **Stage 20 — One previous-session resolver:** Give restore, prepare, run, and
|
||||
dry-run one consistent view of required and optional previous-session inputs,
|
||||
while avoiding ambiguous matches and duplicate downloads.
|
||||
21. **Stage 21 — Strict configuration:** Reject multiple YAML documents, invalid
|
||||
durations, implicit storage backends, and unmet previous-session expectations,
|
||||
while separating configuration tests by responsibility.
|
||||
22. **Stage 22 — Truthful product settings and temp-file ownership:** Remove
|
||||
configuration fields that do nothing, reject unsupported notification
|
||||
settings, and guarantee cleanup of remote-configuration temporary files.
|
||||
23. **Stage 23 — Streaming WhisperX transport:** Stream uploads instead of
|
||||
buffering entire audio files, reject unsupported endpoint schemes, and make
|
||||
retries, cancellation, fake-server recording, and race tests reliable.
|
||||
24. **Stage 24 — Correct prepare/transcribe transitions:** Prevent stale previous
|
||||
data, cancelled or partial transcription work, and duplicate source identity
|
||||
from being recorded as successful current output.
|
||||
25. **Stage 25 — Authoritative output paths:** Require adapters to honor the
|
||||
stage-requested output destination and consolidate duplicate singleton
|
||||
transcript resolution without confusing it with multi-source discovery.
|
||||
26. **Stage 26 — Shared extraction evidence:** Consolidate duplicated
|
||||
extraction-bundle validation into one typed proof while allowing resume and
|
||||
catalog consumers to apply their distinct policies.
|
||||
27. **Stage 27 — Transcript-aware extraction reuse:** Include the direct
|
||||
transcript's identity in extraction freshness checks so changed input cannot
|
||||
reuse stale structured artifacts.
|
||||
28. **Stage 28 — One effective artifact selection:** Resolve configured and
|
||||
explicitly selected artifacts once, then use that same typed set for
|
||||
prerequisites, extraction catalogs, analyze inputs, and execution planning.
|
||||
29. **Stage 29 — Predictable analyze planning:** Represent optional and required
|
||||
analyze inputs explicitly, produce deterministic dependency errors, and give
|
||||
operators correct remediation commands.
|
||||
30. **Stage 30 — Contract cleanup:** Remove dead or misleading interfaces and
|
||||
helpers, move static Audita configuration to its proper owner, and correct
|
||||
stale contract comments.
|
||||
31. **Stage 31 — Enforced automated validation:** Require tests, race checks, vet,
|
||||
builds, and example validation for changes and releases, while consolidating
|
||||
redundant broad tests without losing focused coverage.
|
||||
32. **Stage 32 — Documentation and closure:** Reconcile normative documentation
|
||||
with the completed behavior and verify that every planned remediation has one
|
||||
completed, traceable implementation stage.
|
||||
|
||||
Every stage's purpose was readily determinable from the implementation plan; no
|
||||
stage required an uncertainty note.
|
||||
@@ -1,391 +0,0 @@
|
||||
# Audit Remediation Implementation Plan
|
||||
|
||||
## Purpose and status
|
||||
|
||||
This document is the executable roadmap for remediating the confirmed findings in
|
||||
[`audit-findings.md`](audit-findings.md) and the concrete gaps found during the
|
||||
post-implementation code review. It is written for a `gpt-5.6-terra` coding agent
|
||||
that will implement exactly one pending stage per prompt, in order.
|
||||
|
||||
The audit is an immutable requirements and evidence ledger. Do not edit
|
||||
`audit-findings.md` or reinterpret its accepted findings. Stages 1–32 completed
|
||||
the original audit-remediation plan and are summarized below instead of retaining
|
||||
their now-obsolete implementation instructions. The repository history and audit
|
||||
remain the detailed evidence for that work.
|
||||
|
||||
Stages 33–35 address follow-up review findings in the completed implementation.
|
||||
They are implementation gaps in the original remediation rather than new entries
|
||||
in the immutable audit ledger. They do not require the implementing agent to read
|
||||
the full audit or any audit line range.
|
||||
|
||||
## Stage status and completed summaries
|
||||
|
||||
| Stage | Outcome or pending work | Source | Status |
|
||||
| ---: | --- | --- | --- |
|
||||
| 1 | Aligned architecture policy and filesystem modes with collaborative, group-owned workspaces while preserving private API-key handling. | RSK-004 | Completed |
|
||||
| 2 | Enforced portable safe-segment identifiers and added path/source fuzz coverage. | COR-002, TST-013 | Completed |
|
||||
| 3 | Consolidated crash-durable atomic file replacement behind shared file-operation primitives. | RSK-002, DUP-001, DUP-005 | Completed |
|
||||
| 4 | Added confined, no-follow destination and download/install operations. | COR-003, DUP-003, TST-003 | Completed |
|
||||
| 5 | Confined recursive cleanup and replaced existence-based sentinel locks with OS-held locks. | RSK-003 | Completed |
|
||||
| 6 | Made API-key acquisition private, bounded, regular-file-only, and no-follow. | RSK-010 | Completed |
|
||||
| 7 | Bounded and verified externally produced adapter and stage result files. | RSK-013, TST-007 | Completed |
|
||||
| 8 | Added owned subprocess-tree cancellation, timeout, and forced termination. | RSK-011 | Completed |
|
||||
| 9 | Added streaming secret redaction and bounded subprocess diagnostic capture. | RSK-012 | Completed |
|
||||
| 10 | Confined publish archive reads to validated workspace sources. | COR-005 | Completed |
|
||||
| 11 | Established singular manifest and run identity ownership. | COR-001, TST-006 | Completed |
|
||||
| 12 | Centralized handled terminal-failure persistence and removed competing failure paths. | RSK-001, TST-002, SIM-001, COM-001 | Completed |
|
||||
| 13 | Introduced immutable remote commit manifests, a final current pointer, and an isolated removable legacy reader. | ARC-003 | Completed |
|
||||
| 14 | Switched publishing to immutable commits, deterministic mappings, and a final pointer update. | COR-004, COR-011, DUP-002, TST-004 | Completed |
|
||||
| 15 | Made remote locks generation-safe and rejected non-progressing storage pagination. | RSK-005, RSK-014 | Completed |
|
||||
| 16 | Persisted retryable post-commit cleanup obligations. | COR-006, COR-007 | Completed |
|
||||
| 17 | Bound restore and status to one verified committed snapshot and rejected conflicting state. | COR-008, COR-009, TST-005 | Completed |
|
||||
| 18 | Serialized restore transitions, persisted interrupted-restore state, and made restored paths portable. | RSK-006, RSK-008 | Completed |
|
||||
| 19 | Bound reusable audio cache entries to remote object identity. | RSK-007 | Completed |
|
||||
| 20 | Unified previous-source readiness and eliminated duplicate transfers. | COR-010, EFF-001 | Completed |
|
||||
| 21 | Tightened configuration parsing, validation, value domains, and test expectations. | COR-012–COR-015, TST-011, TST-014 | Completed |
|
||||
| 22 | Made product configuration truthful and assigned lifecycle ownership for downloaded temporary files. | COR-024, RSK-009, ARC-004 | Completed |
|
||||
| 23 | Streamed WhisperX uploads and made its transport bounded, retry-correct, and race-safe. | COR-016, EFF-002, TST-001 | Completed |
|
||||
| 24 | Corrected prepare/transcribe transition, retry, and optional-output semantics. | COR-017–COR-019, TST-008 | Completed |
|
||||
| 25 | Enforced authoritative output paths and shared singleton resolution. | ARC-006, DUP-006 | Completed |
|
||||
| 26 | Centralized typed extraction-bundle evidence. | DUP-007 | Completed |
|
||||
| 27 | Bound extraction reuse to direct transcript identity and explicit freshness evidence. | COR-020, TST-009 | Completed |
|
||||
| 28 | Established one effective artifact set and deterministic catalog bootstrap behavior. | COR-022, ARC-007, DUP-008 | Completed |
|
||||
| 29 | Made analyze input resolution typed, optional, actionable, and deterministic. | COR-021, COR-023, RSK-015, SIM-003, TST-010 | Completed |
|
||||
| 30 | Removed misleading contracts and placed static Audita policy in adapter construction. | ARC-001, ARC-005, SIM-004, COM-003, COM-006 | Completed |
|
||||
| 31 | Enforced repository-wide CI/release validation and streamlined redundant test matrices. | TST-012, TST-015 | Completed |
|
||||
| 32 | Reconciled lifecycle/analyze documentation and closed the original audit traceability inventory. | COM-002, COM-005 | Completed |
|
||||
| 33 | Confine diagnostic log destinations and eliminate pathname-based tail reads. | Follow-up review | Completed |
|
||||
| 34 | Dispose of owned subprocess descendants after natural leader exit. | Follow-up review | Completed |
|
||||
| 35 | Bound remote current-state and lock control-plane reads. | Follow-up review | Completed |
|
||||
|
||||
Completed stages must not be reimplemented wholesale. A pending stage may adjust
|
||||
their code only where its stated remediation requires it.
|
||||
|
||||
## Governing decisions
|
||||
|
||||
The following decisions are settled requirements, not questions for the
|
||||
implementing agent:
|
||||
|
||||
1. Ordinary Narratio and Notarius campaign/session data is deliberately
|
||||
shareable, not private or sensitive. Group-owned, group-writable workspaces are
|
||||
an operational requirement. API keys are the only sensitive data handled by
|
||||
these applications and must remain private.
|
||||
2. On POSIX systems, Narratio-managed ordinary workspace directories and files
|
||||
should converge on setgid `02775` and `0664` respectively, inheriting the
|
||||
workspace's existing group. API-key directories and files must be `0700` and
|
||||
`0600`. Do not add ownership-changing behavior or assume Narratio may `chown`.
|
||||
Windows behavior must preserve the same collaboration/security intent using
|
||||
the platform's available guarantees and explicit operational guidance.
|
||||
3. Remote current-state publication uses an immutable run-scoped commit manifest
|
||||
selected by one final pointer update. Readers may accept coherent legacy state,
|
||||
but all compatibility code must remain isolated behind a clearly named
|
||||
boundary, covered by dedicated tests, and carry an in-code removal comment.
|
||||
New writers must never produce the legacy format.
|
||||
4. Explicit notification no-op is supported. Configured notification backends or
|
||||
recipients are rejected until a real provider exists.
|
||||
5. External-data limits are runaway safeguards, not normal operating limits.
|
||||
Each limit must be a named, centrally discoverable constant in the package that
|
||||
owns the contract and at least an order of magnitude above expected ordinary
|
||||
data. Every limit error must identify the enforcing adapter or contract, the
|
||||
configured byte limit, and what exceeded it. Hard whole-disk protection remains
|
||||
an operations concern implemented with filesystem, service, container, or
|
||||
volume quotas rather than output-root polling.
|
||||
6. Narratio owns the complete process group or job created for every external
|
||||
command. Leader completion does not transfer ownership of surviving
|
||||
descendants. `Run` must not return while an owned descendant can continue
|
||||
running, including when the leader exits successfully or closes its streams.
|
||||
7. Diagnostic tails are derived only from content that passed through the
|
||||
streaming redactor. Diagnostic files use the same confined, no-follow mutation
|
||||
guarantees as other workspace destinations; a pathname must not be reopened to
|
||||
obtain an error tail after execution.
|
||||
8. Remote current pointers, commit manifests, selected session manifests, and
|
||||
lock documents are bounded while being read. Object metadata may reject an
|
||||
oversized value early but cannot replace enforcement against the actual byte
|
||||
stream. Limits for these small control-plane contracts are separate from limits
|
||||
for intentionally large artifact payloads.
|
||||
9. Retain every accepted-risk disposition in the audit. In particular, do not
|
||||
turn abrupt process-death detection into a distributed liveness system, add
|
||||
rollback of arbitrary external side effects, or introduce generic speculative
|
||||
abstractions rejected by the audit.
|
||||
10. CI must run ordinary tests, the full race suite, vet, build, and example/docs
|
||||
checks on every change and release path; shuffled tests run on a schedule. Add
|
||||
native macOS/Windows jobs only when authoritative Woodpecker runner labels and
|
||||
usable runners are available. Do not invent labels. Cross-compilation is
|
||||
useful but does not count as native filesystem or process evidence.
|
||||
|
||||
## Instructions for every pending stage
|
||||
|
||||
For each implementation prompt, the coding agent must:
|
||||
|
||||
1. Read `docs/development.md`, then the current-behavior documents and source
|
||||
files named by that stage. Follow the task-specific reading guide in
|
||||
`docs/development.md`. Stages 33–35 do not require reading
|
||||
`audit-findings.md`; do not reread or modify the immutable audit ledger.
|
||||
2. Inspect the current tree before editing. Earlier stages may have changed names
|
||||
and ownership boundaries. Use the repository knowledge graph first for code
|
||||
discovery and call tracing, then text search for documentation, string
|
||||
literals, configuration, and evidence the graph cannot supply.
|
||||
3. Confirm the worktree state and preserve unrelated user changes. Do not rewrite
|
||||
or amend earlier commits unless explicitly instructed.
|
||||
4. Implement the stage completely, including production code, focused regression
|
||||
tests, platform-specific implementations where applicable, current-behavior
|
||||
documentation, and removal of code made obsolete by the stage. Do not leave a
|
||||
second competing path or defer required tests to a later stage unless this plan
|
||||
explicitly says so.
|
||||
5. Prefer typed, owner-specific contracts and existing package ownership. Keep
|
||||
policy at the owner and reusable mechanism in `internal/fileops`, storage, or
|
||||
the relevant adapter package. Do not create a generic abstraction solely
|
||||
because two call sites look similar.
|
||||
6. Treat path, race, process-lifecycle, and size checks as production behavior.
|
||||
Validation followed by an unsafe pathname operation or unbounded read is not a
|
||||
completed fix. Unsupported safe behavior must fail closed with an actionable
|
||||
error.
|
||||
7. Run focused tests while developing, then run at minimum:
|
||||
|
||||
```sh
|
||||
go test ./...
|
||||
go test -race ./...
|
||||
go vet ./...
|
||||
go build ./...
|
||||
go test ./internal/doccheck
|
||||
go test ./internal/config -run '^TestExamplesLoadAndValidate$'
|
||||
```
|
||||
|
||||
Run the platform build/test checks named by the stage. Do not require live
|
||||
cloud or third-party services.
|
||||
8. Compare the final diff with the stage goal and exit criteria. Update only that
|
||||
stage's row in the status table from `Pending` to `Completed`, adding the
|
||||
implementing commit hash if the workflow supplies one. Do not mark a stage
|
||||
complete while a required check is failing or required behavior is missing.
|
||||
|
||||
## Stage 33 — Confine diagnostic log destinations and eliminate pathname-based tail reads
|
||||
|
||||
**Read first:** `docs/policy/architecture.md`, `docs/policy/testing.md`,
|
||||
`docs/internal/adapters.md`, `docs/internal/workspace.md`, and the current
|
||||
subprocess diagnostic and confined file-operation implementations and tests.
|
||||
|
||||
**Depends on:** Completed Stages 3, 4, and 9.
|
||||
|
||||
**Goal:** Prevent diagnostic log setup from following a replaceable leaf symlink
|
||||
or mutating a file outside the intended workspace, and ensure error diagnostics
|
||||
cannot be redirected to read unrelated or secret content after a command runs.
|
||||
|
||||
**Work:**
|
||||
|
||||
- Replace direct diagnostic `os.Create` use with the established confined,
|
||||
no-follow destination mechanism. Validate the leaf as part of the mutation,
|
||||
reject symlink and non-regular destinations, preserve group-workspace modes, and
|
||||
retain the platform-specific safety guarantees established by Stage 4. A prior
|
||||
ancestor check followed by an ordinary pathname create is not sufficient.
|
||||
- Remove pathname reopening from diagnostic-tail construction. Retain a small,
|
||||
bounded per-stream tail from bytes that have already passed through streaming
|
||||
redaction, preferably in the diagnostic writer itself, and obtain failure
|
||||
diagnostics from that retained state after redaction has been flushed. Do not
|
||||
read the log path again in the error path.
|
||||
- Preserve stdout/stderr file behavior, shared-path behavior, byte caps, split-
|
||||
chunk secret redaction, capture-limit signaling, and group collaboration modes.
|
||||
Do not create an unbounded in-memory copy of either stream.
|
||||
- Keep API-key values out of persisted logs and every returned error even if a
|
||||
workspace peer renames or replaces a diagnostic pathname while the process is
|
||||
running.
|
||||
- Remove obsolete pathname-tail helpers and update current-behavior documentation
|
||||
only if the existing documentation describes the replaced mechanism.
|
||||
|
||||
**Tests and exit criteria:** Add adversarial tests proving that an existing log
|
||||
leaf symlink is rejected without truncating its target; a non-regular leaf is
|
||||
rejected; pathname replacement after the log is opened cannot inject foreign
|
||||
content into a returned error; and a known credential remains absent from files
|
||||
and errors across split writes. Cover separate and shared stdout/stderr paths,
|
||||
exact diagnostic caps, and cap-plus-one behavior. Run focused subprocess and
|
||||
file-operation tests under `-race`, plus Linux, macOS, and Windows cross-builds for
|
||||
affected packages. Native platform tests remain conditional on available runners.
|
||||
|
||||
**Status:** Completed.
|
||||
|
||||
## Stage 34 — Dispose of owned subprocess descendants after natural leader exit
|
||||
|
||||
**Read first:** `docs/policy/architecture.md`, `docs/policy/testing.md`,
|
||||
`docs/internal/adapters.md`, the subprocess run/process-tree implementations for
|
||||
Unix and Windows, and all process-tree test helpers.
|
||||
|
||||
**Depends on:** Completed Stage 8 and Stage 33.
|
||||
|
||||
**Goal:** Close the lifecycle gap in which `cmd.Wait` can complete before a
|
||||
descendant in Narratio's process group or Windows job exits, allowing that
|
||||
descendant to outlive `Run` after either a successful or failed leader exit.
|
||||
|
||||
**Work:**
|
||||
|
||||
- Route the normal `cmd.Wait` completion branch through bounded owned-tree
|
||||
disposal before `Run` returns. Apply the ownership rule for successful exits,
|
||||
non-zero exits, and `exec.ErrWaitDelay`, not only cancellation, timeout, or
|
||||
diagnostic-cap paths.
|
||||
- On Unix, detect and gracefully terminate any remaining process-group members,
|
||||
wait only for the existing bounded grace period, then forcefully terminate
|
||||
survivors. Treat an already-empty group as success and avoid imposing the grace
|
||||
delay on ordinary commands with no descendants.
|
||||
- On Windows, preserve job-object kill-on-close semantics and ensure natural
|
||||
leader completion closes/disposes the job before return. Keep handles and
|
||||
goroutines bounded on every exit path.
|
||||
- Consolidate terminal cleanup so races among context cancellation, capture-limit
|
||||
signaling, and leader completion cannot skip disposal or produce unsafe double
|
||||
ownership. Preserve timeout/cancellation classification and join actionable
|
||||
cleanup failures without hiding the original command result.
|
||||
- Treat a descendant that deliberately leaves the owned process group or job as
|
||||
outside this stage; do not build a system-wide process discovery mechanism.
|
||||
|
||||
**Tests and exit criteria:** Extend the real subprocess helper with cases where a
|
||||
leader exits zero while a descendant (a) retains inherited streams and (b)
|
||||
redirects or closes them, plus a non-zero leader case. Prove `Run` completes
|
||||
within bounded time and no descendant reaches a delayed sentinel after return.
|
||||
Retain cancellation, timeout, and capture-limit coverage and run the subprocess
|
||||
suite repeatedly, shuffled, and under `-race`. Cross-build the affected packages
|
||||
for Linux, macOS, and Windows; run native platform tests only where runners are
|
||||
actually available.
|
||||
|
||||
**Status:** Completed.
|
||||
|
||||
## Stage 35 — Bound remote current-state and lock control-plane reads
|
||||
|
||||
**Read first:** `docs/policy/architecture.md`, `docs/policy/testing.md`,
|
||||
`docs/internal/storage.md`, `docs/internal/artifacts.md`,
|
||||
`docs/internal/command-restore.md`, the current-state commit reader, remote lock
|
||||
loader, object-store contract and implementations, and their stateful fakes and
|
||||
tests.
|
||||
|
||||
**Depends on:** Completed Stages 13–17.
|
||||
|
||||
**Goal:** Prevent corrupt, misconfigured, or hostile object storage from forcing
|
||||
unbounded temporary-disk or memory consumption before small remote control-plane
|
||||
objects can be parsed or verified.
|
||||
|
||||
**Work:**
|
||||
|
||||
- Inventory the new-protocol read paths for the mutable current pointer, immutable
|
||||
commit manifest, selected session manifest, and remote lock document. Include
|
||||
the isolated legacy current-state reader only where it accepts equivalent small
|
||||
control objects. Do not apply these limits to intentionally large published
|
||||
artifact payloads.
|
||||
- Define generous, documented, centrally discoverable constants in the packages
|
||||
that own each control-plane contract. Use separate policy constants where the
|
||||
credible sizes differ; do not embed numeric limits at call sites or replace the
|
||||
owner policies with one unexplained global limit.
|
||||
- Stream each object through an enforcing `limit+1` read before parsing or
|
||||
retaining it. Reject oversized metadata before transfer when available, but
|
||||
still enforce the limit against actual bytes because metadata can be missing,
|
||||
stale, or inaccurate. Avoid downloading these small objects unboundedly to a
|
||||
temporary file and then calling `os.ReadFile` or `io.ReadAll`.
|
||||
- Preserve immutable commit checksum, declared-size, generation/ETag, identity,
|
||||
and pointer-selection validation. Ensure the metadata used for validation
|
||||
belongs to the opened object version; do not weaken coherence while removing
|
||||
the temporary download path.
|
||||
- Every over-limit error must identify the responsible control-plane contract,
|
||||
object category, key or safe storage context, and configured byte limit without
|
||||
including credentials. Maintain cancellation and reader-close behavior on all
|
||||
exits.
|
||||
- Factor a bounded storage-read mechanism only if it has a clear storage-layer
|
||||
invariant; retain the owner-specific constants and errors in the artifact and
|
||||
remote-lock owners.
|
||||
|
||||
**Tests and exit criteria:** Add exact-limit and limit-plus-one cases for each
|
||||
object category, including inaccurate/absent size metadata, short reads,
|
||||
cancellation, malformed JSON/YAML, checksum or generation mismatch, and readers
|
||||
that verify they are closed. Prove oversized current-state downloads do not create
|
||||
unbounded temporary files and oversized lock documents are rejected without
|
||||
unbounded allocation. Retain coherent legacy-reader tests and stateful S3/fake
|
||||
ordering and conditional-write tests. Run focused storage, artifacts, app, restore,
|
||||
and publish tests under `-race`, followed by the complete repository validation.
|
||||
|
||||
**Status:** Completed.
|
||||
|
||||
## Finding traceability inventory
|
||||
|
||||
This inventory remains the closed traceability record for Stages 1–32. A pending
|
||||
follow-up stage may strengthen the implementation of an original stage, but does
|
||||
not change the immutable finding disposition or create a second audit finding.
|
||||
|
||||
| Finding(s) | Primary stage |
|
||||
| --- | ---: |
|
||||
| COR-001 | 11 |
|
||||
| COR-002 | 2 |
|
||||
| COR-003 | 4 |
|
||||
| COR-004 | 14 |
|
||||
| COR-005 | 10 |
|
||||
| COR-006, COR-007 | 16 |
|
||||
| COR-008, COR-009 | 17 |
|
||||
| COR-010 | 20 |
|
||||
| COR-011 | 14 |
|
||||
| COR-012, COR-013, COR-014, COR-015 | 21 |
|
||||
| COR-016 | 23 |
|
||||
| COR-017, COR-018, COR-019 | 24 |
|
||||
| COR-020 | 27 |
|
||||
| COR-021, COR-023 | 29 |
|
||||
| COR-022 | 28 |
|
||||
| COR-024 | 22 |
|
||||
| RSK-001 | 12 |
|
||||
| RSK-002 | 3 |
|
||||
| RSK-003 | 5 |
|
||||
| RSK-004 | 1 |
|
||||
| RSK-005 | 15 |
|
||||
| RSK-006, RSK-008 | 18 |
|
||||
| RSK-007 | 19 |
|
||||
| RSK-009 | 22 |
|
||||
| RSK-010 | 6 |
|
||||
| RSK-011 | 8 |
|
||||
| RSK-012 | 9 |
|
||||
| RSK-013 | 7 |
|
||||
| RSK-014 | 15 |
|
||||
| RSK-015 | 29 |
|
||||
| EFF-001 | 20 |
|
||||
| EFF-002 | 23 |
|
||||
| ARC-001 | 30 |
|
||||
| ARC-003 | 13 |
|
||||
| ARC-004 | 22 |
|
||||
| ARC-005 | 30 |
|
||||
| ARC-006 | 25 |
|
||||
| ARC-007 | 28 |
|
||||
| TST-001 | 23 |
|
||||
| TST-002 | 12 |
|
||||
| TST-003 | 4 |
|
||||
| TST-004 | 14 |
|
||||
| TST-005 | 17 |
|
||||
| TST-006 | 11 |
|
||||
| TST-007 | 7 |
|
||||
| TST-008 | 24 |
|
||||
| TST-009 | 27 |
|
||||
| TST-010 | 28 |
|
||||
| TST-011, TST-014 | 21 |
|
||||
| TST-012, TST-015 | 31 |
|
||||
| TST-013 | 2 |
|
||||
| DUP-001, DUP-005 | 3 |
|
||||
| DUP-002 | 14 |
|
||||
| DUP-003 | 4 |
|
||||
| DUP-006 | 25 |
|
||||
| DUP-007 | 26 |
|
||||
| DUP-008 | 28 |
|
||||
| SIM-001 | 12 |
|
||||
| SIM-003 | 29 |
|
||||
| SIM-004 | 30 |
|
||||
| COM-001 | 12 |
|
||||
| COM-002, COM-005 | 32 |
|
||||
| COM-003, COM-006 | 30 |
|
||||
|
||||
The remaining confirmed IDs have non-independent dispositions and must not receive
|
||||
separate implementation work: ARC-002 and COM-004 are merged into COM-002;
|
||||
SIM-002 is merged into DUP-007; DUP-004 is rejected. Their audit rationale remains
|
||||
authoritative.
|
||||
|
||||
## Follow-up review traceability
|
||||
|
||||
| Review finding | Remediation stage | Source |
|
||||
| --- | ---: | --- |
|
||||
| Diagnostic leaf symlinks and pathname-reopened tails bypass confinement/redaction guarantees | 33 | Post-implementation review |
|
||||
| Unix descendants survive natural leader exit | 34 | Post-implementation review |
|
||||
| Remote current-state and lock control objects are read without acquisition limits | 35 | Post-implementation review |
|
||||
|
||||
## Open Questions
|
||||
|
||||
None. The remediation behavior and ownership decisions are specified above.
|
||||
Native macOS/Windows runtime validation remains contingent on external runner
|
||||
availability, but cross-build requirements and the rule against inventing runner
|
||||
labels make implementation decision-complete.
|
||||
@@ -5,7 +5,6 @@ import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -66,26 +65,6 @@ func TestReleaseWorkflowRequiresValidation(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRoadmapTraceabilityOwnersAreComplete(t *testing.T) {
|
||||
root := repositoryRoot(t)
|
||||
data, err := os.ReadFile(filepath.Join(root, "docs", "roadmap", "implementation.md"))
|
||||
if err != nil {
|
||||
t.Fatalf("read implementation roadmap: %v", err)
|
||||
}
|
||||
|
||||
statuses := roadmapSummaryStatuses(t, string(data))
|
||||
checked := 0
|
||||
for _, owner := range roadmapTraceabilityOwners(t, string(data)) {
|
||||
checked++
|
||||
if statuses[owner] != "Completed" {
|
||||
t.Errorf("traceability owner %d has status %q, want Completed", owner, statuses[owner])
|
||||
}
|
||||
}
|
||||
if checked == 0 {
|
||||
t.Fatal("roadmap traceability inventory has no owners")
|
||||
}
|
||||
}
|
||||
|
||||
func repositoryRoot(t *testing.T) string {
|
||||
t.Helper()
|
||||
root, err := filepath.Abs(filepath.Join("..", ".."))
|
||||
@@ -145,53 +124,6 @@ func relativeToRoot(root, path string) string {
|
||||
return relative
|
||||
}
|
||||
|
||||
func roadmapSummaryStatuses(t *testing.T, document string) map[int]string {
|
||||
t.Helper()
|
||||
statuses := map[int]string{}
|
||||
for _, line := range strings.Split(document, "\n") {
|
||||
fields := strings.Split(line, "|")
|
||||
if len(fields) != 6 {
|
||||
continue
|
||||
}
|
||||
owner, err := strconv.Atoi(strings.TrimSpace(fields[1]))
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
statuses[owner] = strings.TrimSpace(fields[4])
|
||||
}
|
||||
if len(statuses) == 0 {
|
||||
t.Fatal("roadmap summary has no status entries")
|
||||
}
|
||||
return statuses
|
||||
}
|
||||
|
||||
func roadmapTraceabilityOwners(t *testing.T, document string) []int {
|
||||
t.Helper()
|
||||
const inventoryHeading = "## Finding traceability inventory"
|
||||
const nextHeading = "## Open Questions"
|
||||
start := strings.Index(document, inventoryHeading)
|
||||
if start < 0 {
|
||||
t.Fatal("roadmap traceability inventory is missing")
|
||||
}
|
||||
inventory := document[start+len(inventoryHeading):]
|
||||
if end := strings.Index(inventory, nextHeading); end >= 0 {
|
||||
inventory = inventory[:end]
|
||||
}
|
||||
|
||||
owners := []int{}
|
||||
for _, line := range strings.Split(inventory, "\n") {
|
||||
fields := strings.Split(line, "|")
|
||||
if len(fields) != 4 {
|
||||
continue
|
||||
}
|
||||
owner, err := strconv.Atoi(strings.TrimSpace(fields[2]))
|
||||
if err == nil {
|
||||
owners = append(owners, owner)
|
||||
}
|
||||
}
|
||||
return owners
|
||||
}
|
||||
|
||||
type woodpeckerWorkflow struct {
|
||||
Steps map[string]woodpeckerStep `yaml:"steps"`
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user