Finalize codebase audit findings
This commit is contained in:
@@ -1,10 +1,47 @@
|
||||
# Codebase Audit Findings
|
||||
|
||||
Status: in progress
|
||||
Status: complete
|
||||
|
||||
This document is the working ledger and final report for the audit defined by
|
||||
the [audit plan](audit-plan.md) and [audit sequence](audit-sequence.md). The
|
||||
audit is investigative: candidate findings below are not remediation changes.
|
||||
audit is investigative: findings below are not remediation changes.
|
||||
The classifications and backlog describe proposed future work; no production
|
||||
change is included in this report.
|
||||
|
||||
## Executive Assessment
|
||||
|
||||
Narratio has a clear stage-oriented architecture, deterministic normal
|
||||
execution, strong package ownership for most parsing and protocol behavior, and
|
||||
substantial focused coverage of ordinary lifecycle, adapter, artifact, and
|
||||
workflow paths. The session manifest is consistently treated as progress
|
||||
authority, the canonical stage order is explicit, run-local outputs are usually
|
||||
validated before canonical materialization, publish writes its pointer last,
|
||||
restore installs the manifest last, and the default test suite is fast,
|
||||
offline, and credential-free. The audit found no production dependency
|
||||
reversal, generic workflow-engine drift, unsupported live-service dependency in
|
||||
tests, or broad need for new abstraction.
|
||||
|
||||
Those strengths do not close several consequential boundary defects. The most
|
||||
urgent findings are symlink-following mutation/deletion and publish reads
|
||||
(`COR-003`, `COR-005`), restore accepting state not bound to the selected
|
||||
remote commit (`COR-008`), unsafe identity components crossing local and remote
|
||||
namespaces (`COR-002`), and secret/private-data exposure through permissive
|
||||
modes, filesystem-secret reads, and subprocess diagnostics (`RSK-004`,
|
||||
`RSK-010`, `RSK-012`). Remote replacement publish can also invalidate the
|
||||
previous readable commit before its final pointer write (`COR-004`). These are
|
||||
high-consequence integrity, confidentiality, destructive-operation, and
|
||||
recovery boundaries even though several require an uncommon failure, hostile
|
||||
filesystem state, or multi-user deployment.
|
||||
|
||||
The broader result is not a recommendation for wholesale redesign. The
|
||||
preferred remediation is a dependency-ordered set of narrow owners: establish
|
||||
confined filesystem and durable-write capabilities; make manifest identity and
|
||||
terminal persistence singular; repair publish/restore commit authority; then
|
||||
correct configuration, adapter, stage, and analyze contracts. Structural,
|
||||
test, efficiency, and comment work should follow or accompany those roots only
|
||||
where it makes the corrected invariant easier to preserve. The ordered backlog
|
||||
below consolidates 77 confirmed IDs into coherent workstreams so remediation
|
||||
does not repeat discovery or fix the same mechanism in several callers.
|
||||
|
||||
## Audit Identity And Baseline
|
||||
|
||||
@@ -110,7 +147,7 @@ These are prioritization signals, not findings:
|
||||
| 10: analyze and dependencies | complete | All five source-policy families, seven built-in catalog entries, configured-artifact execution/reuse, dependency validation/order, previous-cache locality, lifecycle, and publish selection were traced as one slice. The exact focused command passed. Confirmed `COR-021` through `COR-024` and `RSK-015`; added `ARC-007`, `DUP-008`, `SIM-003`, `COM-005`, and `TST-010`; and refined `ARC-001`, `ARC-002`, `RSK-013`, and scenario 9. |
|
||||
| 11: maintainability | complete | Production graph metrics, change coupling, dead-code/static patterns, all structural candidates, efficiency workloads, comments, dependencies, and platform assumptions reviewed. Confirmed the narrow `ARC`, `DUP`, and `SIM` corrections recorded below, merged lifecycle wording and extraction proof candidates, rejected generic abstractions and micro-optimizations, added `SIM-004` and `COM-006`, and passed the full normal test suite and vet. |
|
||||
| 12: test policy | complete | All 749 tests were inventoried by behavior owner and consequential risk; every prior `TST` candidate was classified, five suite-wide candidates were added, and coverage, doubles, helpers, determinism, offline behavior, runtime, fuzzing, and automation were assessed. The prescribed shuffled run exposed `TST-011`; the prescribed race run reproduced only `TST-001`. |
|
||||
| 13: synthesis | not_started | No final ranking or accepted-risk decisions yet. |
|
||||
| 13: synthesis | complete | Revalidated the pinned implementation boundary, reconciled all registers and matrices, retained 77 confirmed IDs with four merged/rejected dispositions, recorded positive conclusions, dimensions and accepted risks, and produced an 11-workstream dependency-ordered remediation backlog. Report links and document integrity were validated; no production change occurred. |
|
||||
|
||||
## Area Coverage And Ownership
|
||||
|
||||
@@ -313,18 +350,18 @@ terminalization/reconciliation harder to test.
|
||||
|
||||
## Cross-Boundary Scenario Assignments
|
||||
|
||||
| Scenario | Primary audit stage | Supporting packages and focused tests |
|
||||
| --- | --- | --- |
|
||||
| 1. Success becomes non-resumable, rerun fails, later reuse decision | 2 | `internal/app`, `internal/manifest`, `stage.ResumeValidator`; `runner_test.go`, `extract_lifecycle_test.go`, manifest transition tests |
|
||||
| 2. Forced/changed upstream outcome with succeeded, self-skipped, disabled downstream | 2 | `internal/app`, `internal/stage`, `internal/manifest`; run-control, runner, extraction-lifecycle tests; disabled-stage detail revisited in 8 |
|
||||
| 3. Extraction bundle followed by configuration/transitive-input change | 9 | Extract/resume, Notarius adapter, artifacts/fileops tests; downstream resolution revisited in 10 |
|
||||
| 4. Published/restored/prepared previous state consumed locally by analyze | 5 | `internal/app`, `internal/previouscache`, `internal/audio`, `internal/artifacts`; restore/prepare tests; analyze consumption revisited in 10 |
|
||||
| 5. Publish failure at every upload boundary, then status/restore/retry | 4 | Publish stage, storage fake/adapter, app status/restore; publish and operator-helper tests; restore interpretation revisited in 5 |
|
||||
| 6. Restore identical/conflict/unsafe/cache/pre-manifest-install cases | 5 | Restore discovery/plan/execute/report, artifacts, previouscache, audio; restore test suite |
|
||||
| 7. Cleanup after skipped/failed/locked/partial/committed publish | 4 | Publish metadata, post-publish cleanup, cleanup targets, pathsafe; publish/cleanup tests |
|
||||
| 8. Cancellation through workers, HTTP, subprocess, storage, manifests | 7 | Adapter and subprocess tests; transcribe/stage tests in 8; runner reporting in 2 |
|
||||
| 9. Disabled/unselected/reused/generated/extraction/previous source then publish filtering | 10 | Analyze, artifact catalog/resolver/policy, publish tests; config ownership in 6 and publish result in 4 |
|
||||
| 10. Concurrent same-session invocation and lock cleanup failures | 3 | Runner lock lifetime in 2; local artifact store, path/file cleanup and lock tests in 3 |
|
||||
| Scenario | Primary audit stage | Final conclusion | Principal findings or accepted boundary |
|
||||
| --- | --- | --- | --- |
|
||||
| 1. Success becomes non-resumable, rerun fails, later reuse decision | 2 | Session reuse is conservative and correct; handled validation errors leave inaccurate invocation audit state. | `RSK-001`, `TST-002`; abrupt-death residue is accepted below after handled errors are repaired. |
|
||||
| 2. Forced/changed upstream outcome with succeeded, self-skipped, disabled downstream | 2 | Canonical invalidation is sound; durable success, self-skip, and idempotent skip are distinct, but documentation blurs them. | `COM-002`; no production lifecycle defect. |
|
||||
| 3. Extraction bundle followed by configuration/transitive-input change | 9 | Configured values are fingerprinted and external same-path changes are operator-forced; the direct trimmed input is incorrectly omitted. | `COR-020`; accepted external-force boundary below. |
|
||||
| 4. Published/restored/prepared previous state consumed locally by analyze | 5 | Analyze is correctly local-only, but commit binding, readiness, source mapping, and effective selection are incomplete. | `COR-008`, `COR-010`, `COR-011`, `COR-022`. |
|
||||
| 5. Publish failure at every upload boundary, then status/restore/retry | 4 | Pointer-last order alone is insufficient because the fixed manifest replaces the prior pair and readers do not share one strict authority rule. | `COR-004`, `COR-008`, `TST-004`. |
|
||||
| 6. Restore identical/conflict/unsafe/cache/pre-manifest-install cases | 5 | Ordinary deterministic planning is strong; committed scope, directory force, coherent transition, audio identity, and foreign paths remain unsafe. | `COR-008`, `COR-009`, `RSK-006` through `RSK-008`, `TST-005`. |
|
||||
| 7. Cleanup after skipped/failed/locked/partial/committed publish | 4 | Gates correctly require an explicit commit, but automatic cleanup is not durable/retryable and path mutation is not symlink-confined. | `COR-003`, `COR-006`, `COR-007`. |
|
||||
| 8. Cancellation through workers, HTTP, subprocess, storage, manifests | 7 | Direct operations generally receive cancellation and release resources; transcribe can report partial success and subprocess descendants can survive. | `COR-018`, `RSK-011`, `RSK-014`, `EFF-002`. |
|
||||
| 9. Disabled/unselected/reused/generated/extraction/previous source then publish filtering | 10 | Source families and publish filtering are deterministic, but effective selection does not consistently drive validation/prerequisites and some optional/passthrough contracts fail. | `COR-021` through `COR-024`, `RSK-015`, `ARC-007`. |
|
||||
| 10. Concurrent same-session invocation and lock cleanup failures | 3 | Live local exclusion spans the full mutation lifetime; stale/release failures and remote lock snapshots remain operational risks. | `RSK-003`, `RSK-005`, `TST-003`. |
|
||||
|
||||
Scenario 3 is resolved. The fingerprint deterministically observes the resolved
|
||||
executable and top-level config *paths*, pipeline ID, normalized timeout,
|
||||
@@ -1542,6 +1579,153 @@ other sorting, copying, map/slice allocation, serialization, adapter
|
||||
construction, remote-call, filesystem-pass, or goroutine/channel pattern had a
|
||||
credible workload large enough to justify a performance finding.
|
||||
|
||||
## Audit Completion And Final Disposition
|
||||
|
||||
The implementation, tests, and canonical current-behavior documentation remain
|
||||
identical to the pinned audited revision. `git diff` from
|
||||
`74e2d21de5fb2ada0be5ef3fe9333e0d48ac7fb3` through the Stage 12 entry revision
|
||||
contains only this ledger and its two audit specifications; there is no Go,
|
||||
module, example, policy, CLI, configuration, operations, integration, or
|
||||
internal-document change. Stage 13 re-ran graph ownership searches across the
|
||||
runner, canonical paths, file operations, publish/current state, restore,
|
||||
configuration, adapters, ordinary stages, extraction, analyze, and their
|
||||
focused tests. Every detailed finding below still resolves to its recorded
|
||||
implementation owner and consumer boundary. No item rests on a metric alone,
|
||||
and no confirmed item was downgraded or rejected during revalidation.
|
||||
|
||||
### Completion criteria
|
||||
|
||||
| Criterion | Final evidence and conclusion |
|
||||
| --- | --- |
|
||||
| Every inspection area reviewed | All 15 area-ledger rows are `reviewed`; Stages 2-12 record contracts, production owners, focused tests, commands, findings, and explicit no-finding conclusions. |
|
||||
| Lifecycle and scenarios concluded | Every lifecycle outcome is source-backed for both manifests. The ten-scenario matrix above now records a final conclusion and root finding or accepted boundary for each row. |
|
||||
| Duplication classified | `DUP-001` through `DUP-003` and `DUP-005` through `DUP-008` are narrow shared mechanisms/policies; `DUP-004` is rejected because its accepted language differs. Broad stage, workflow, resolver, validator, adapter, fake, and manifest abstractions are explicitly rejected. |
|
||||
| Simplification and efficiency bounded | `SIM-001`, `SIM-003`, and `SIM-004` name smaller owners without hiding policy; `SIM-002` merges into `DUP-007`. `EFF-001` and `EFF-002` name representative byte/latency/memory measurements; no other performance claim survived workload review. |
|
||||
| Comments preserve rationale | `COM-001` owns ledger ordering rationale; `COM-002` owns durable no-output terminology; `COM-003`, `COM-005`, and `COM-006` repair verified stale/incomplete claims. `COM-004` merges into `COM-002`. |
|
||||
| Test suite assessed by risk | The final matrix covers integrity, destructive actions, compatibility, security, concurrency, idempotency, recovery, cancellation, partial success, redundancy, doubles, helpers, coverage, fuzzing, determinism, offline behavior, runtime, and automation. |
|
||||
| Findings deduplicated and ranked | 77 IDs remain confirmed. Four stable IDs are non-independent dispositions: `ARC-002` and `COM-004` merge into `COM-002`, `SIM-002` merges into `DUP-007`, and `DUP-004` is rejected. The backlog below is authoritative for dependency/risk order. |
|
||||
| Questions, accepted risks, and limitations explicit | The final decision table after the registers assigns an owner and safe interim boundary to every unresolved contract family, followed by intentionally accepted risks and evidence limitations. |
|
||||
|
||||
### Positive conclusions
|
||||
|
||||
- The canonical stage registry and planner are deterministic, and force,
|
||||
changed outcomes, self-skip, ordinary success, failure, and retry invalidate
|
||||
or preserve downstream state conservatively.
|
||||
- Session state is consistently the cross-invocation authority; individual
|
||||
atomic saves preserve the previous file on ordinary pre-rename failure, and
|
||||
terminal session-first ordering preserves safe reuse even when the audit
|
||||
ledger becomes inaccurate.
|
||||
- Directory promotion strongly validates and identity-checks its source,
|
||||
rejects replacement of an existing immutable destination, syncs its durable
|
||||
tree, and fails explicitly on unsupported platforms.
|
||||
- Publish uses deterministic upload ordering and writes the current pointer
|
||||
last. Manual cleanup is explicit, scoped, dry-runnable, cache-preserving by
|
||||
default, and correctly independent of automatic postcommit policy.
|
||||
- Restore planning is deterministic, confines lexical remote mappings, keeps
|
||||
dry-run free of durable local writes, validates downloads before install, and
|
||||
installs the session manifest last. Analyze consumes previous-session state
|
||||
locally and never performs an implicit remote read.
|
||||
- Configuration rejects unknown fields, centralizes defaults, validates
|
||||
maintained examples, keeps ordinary secrets indirect, and conditionally
|
||||
composes external adapters rather than contacting live services at startup.
|
||||
- Adapter protocol suites protect command arguments, schemas, retries,
|
||||
cancellation at direct boundaries, error adaptation, and deterministic
|
||||
outputs. Stages generally validate run-local results before canonical
|
||||
materialization and record artifacts separately from diagnostics.
|
||||
- Production dependencies are active and confined to their boundary. The audit
|
||||
found no generic workflow-engine need, broad dependency-direction inversion,
|
||||
additional material hot path, or reason to chase uniform coverage.
|
||||
- The normal suite passes in about 3.5 seconds without live services, paid APIs,
|
||||
ambient credentials, or fixed external ports. Most tests use real temporary
|
||||
files, loopback HTTP, or the current test binary and assert observable state
|
||||
rather than private choreography.
|
||||
|
||||
### Final prioritization dimensions
|
||||
|
||||
The 42 detailed `COR`, `RSK`, `EFF`, and confirmed `ARC-004` entries already
|
||||
record impact, likelihood, confidence, scope, owner, tests, and dependencies
|
||||
separately; Stage 13 revalidation did not change those ratings. The following
|
||||
compact register supplies the same dimensions for the confirmed structural,
|
||||
test, and clarity entries. “Inherited” impact means the item is required to
|
||||
make its named correctness/security root durable rather than representing a
|
||||
second production defect.
|
||||
|
||||
| IDs | Impact | Likelihood | Confidence | Scope |
|
||||
| --- | --- | --- | --- | --- |
|
||||
| `ARC-001` | Low current; medium future contract misuse | Low until a consumer appears | High | Small removal |
|
||||
| `ARC-003` | Medium remote/local state ambiguity | Low-to-moderate as consumers grow | High | Small-to-medium model/protocol clarification |
|
||||
| `ARC-005` | Medium adapter/test contract drift | Moderate during adapter change | High | Small request/constructor change |
|
||||
| `ARC-006` | High run-local output-authority risk | Low currently; moderate with a divergent adapter | High | Medium shared contract and caller repair |
|
||||
| `ARC-007` | High effective-selection correctness | Moderate for explicit selection | High | Medium app/config/artifact planning change |
|
||||
| `DUP-001`, `DUP-003`, `DUP-005` | High durability/security drift inherited from `RSK-002`/`COR-003` | Moderate during shared repair | High | Medium shared fileops capability and caller migration |
|
||||
| `DUP-002` | Medium run-identity drift | Low-to-moderate | High | Tiny path-owner correction |
|
||||
| `DUP-006` | Medium transcript-source drift | Moderate during source changes | High | Small-to-medium artifact resolver migration |
|
||||
| `DUP-007` | High extraction evidence drift | Moderate during compatibility/security changes | High | Medium typed evidence owner |
|
||||
| `DUP-008` | High analyze/publish/operator catalog drift | Moderate as source families change | High | Medium shared bootstrap with explicit caller policy |
|
||||
| `SIM-001` | Medium audit/persistence correctness support | Moderate on new error paths | High | Small-to-medium runner extraction |
|
||||
| `SIM-003` | Medium maintainability and ordering support | Moderate during analyze changes | High | Medium typed plan/context refactor |
|
||||
| `SIM-004` | Low dead-code cost | Certain but harmless | High | Tiny deletion |
|
||||
| `COM-001` | Medium risk of weakening ledger order | Moderate during runner repair | High | Tiny comment after `SIM-001` |
|
||||
| `COM-002`, `COM-005` | Medium operator/developer lifecycle misunderstanding | Present in current docs | High | Small documentation correction |
|
||||
| `COM-003`, `COM-006` | Low stale-contract friction | Present in current comments | High | Tiny comment deletion/update |
|
||||
| `TST-001`, `TST-011` | Medium loss of diagnostic determinism | Certain under race/repetition runs | High, reproduced | Small fake/environment cleanup |
|
||||
| `TST-002` through `TST-010`, `TST-013` | Inherited critical/high confidence for named integrity, recovery, security, and concurrency roots | Same plausible scenarios as linked findings | High after cross-layer inventory | Medium distributed regression additions, each at its named owner |
|
||||
| `TST-012` | High release-validation exposure | Ongoing on every unvalidated change/tag | High | Small-to-medium automation addition |
|
||||
| `TST-014`, `TST-015` | Low-to-medium recurring maintenance friction | High during legitimate config/stage changes | High | Medium test consolidation |
|
||||
|
||||
### Dependency-ordered remediation backlog
|
||||
|
||||
This order is authoritative over the category-number order used by the detailed
|
||||
registers. Regression tests named by a workstream should land with that repair;
|
||||
standalone test-suite and documentation cleanup follows the owning behavior.
|
||||
|
||||
1. Establish safe identity, filesystem, secret, and durable-write foundations:
|
||||
`COR-002`, `COR-003`, `COR-005`, `RSK-002`, `RSK-004`, `RSK-010`,
|
||||
`RSK-012`, and `RSK-013`, together with `DUP-001`, `DUP-003`, `DUP-005`,
|
||||
`TST-003`, the relevant `TST-007` cases, and `TST-013`. This capability must
|
||||
exist before caller-specific restore, cleanup, adapter, or promotion fixes.
|
||||
2. Make manifest identity and terminal persistence singular: `COR-001`,
|
||||
`RSK-001`, `SIM-001`, `COM-001`, `TST-002`, and `TST-006`. Preserve session
|
||||
authority and visible running-versus-terminal save order; do not introduce a
|
||||
generic lifecycle framework or promise cross-file atomicity.
|
||||
3. Repair remote publication and postcommit cleanup: `COR-004`, `COR-006`,
|
||||
`COR-007`, `RSK-005`, `ARC-003`, `DUP-002`, and `TST-004`. Use one immutable
|
||||
commit model and stateful store tests; retain pointer-last semantics and make
|
||||
cleanup evidence durable/retryable.
|
||||
4. Bind restore and previous state to that commit model: `COR-008` through
|
||||
`COR-011`, `RSK-006` through `RSK-009`, `TST-005`, and then `EFF-001`.
|
||||
Reuse the safe filesystem capability and current-state truth table rather
|
||||
than creating restore-only variants.
|
||||
5. Make configuration and composition truthful before execution: `COR-012`
|
||||
through `COR-016`, `COR-024`, and `ARC-004`, with `TST-011` and `TST-014`.
|
||||
Resolve the storage, notification, previous-session, and Scriptorium contract
|
||||
decisions explicitly; reject unsupported values rather than guessing.
|
||||
6. Complete transport liveness and bounded resource behavior: `RSK-011`,
|
||||
`RSK-014`, `EFF-002`, `TST-001`, and the remaining `TST-007` cases. Reuse
|
||||
the step-1 output/redaction owner and keep platform process mechanics in the
|
||||
shared launcher.
|
||||
7. Correct ordinary-stage and extraction transitions: `COR-017` through
|
||||
`COR-020`, `ARC-006`, `DUP-006`, `DUP-007`, `TST-008`, and `TST-009`.
|
||||
Preserve explicit stage policy while sharing only artifact resolution and
|
||||
typed extraction evidence.
|
||||
8. Unify effective analyze selection and dependency behavior: `COR-021`
|
||||
through `COR-023`, `RSK-015`, `ARC-007`, `DUP-008`, `SIM-003`, and
|
||||
`TST-010`. Coordinate `COR-024` from step 5 rather than inventing wire
|
||||
semantics in this refactor.
|
||||
9. Perform remaining architectural and structural cleanup: `ARC-001`,
|
||||
`ARC-005`, `SIM-004`, and any now-obsolete wrapper code. These are
|
||||
independently small but should not distract from data-safety work.
|
||||
10. Enforce and streamline the suite: `TST-012`, then `TST-014` and `TST-015`.
|
||||
Keep focused owners and representative assembled workflows; add the race
|
||||
job only after `TST-001` makes its signal trustworthy.
|
||||
11. Apply documentation/comment repairs after their contracts settle:
|
||||
`COM-002`, `COM-003`, `COM-005`, and `COM-006`. Update canonical current-
|
||||
behavior documents in the same changes that implement the decisions.
|
||||
|
||||
`EFF-001` and `EFF-002` deliberately follow their correctness owners and require
|
||||
the measurements stated in their detailed entries. No other performance work
|
||||
should be added to this backlog without a representative workload.
|
||||
|
||||
## Confirmed Findings
|
||||
|
||||
### `COR-001`: session identity initialization preserves stale invocation paths and accepts conflicting identity
|
||||
@@ -2399,8 +2583,9 @@ credible workload large enough to justify a performance finding.
|
||||
boundary cases. Filesystem crash durability itself remains a Stage 3 concern.
|
||||
- Dependencies: `SIM-001` may provide one failure-finalization path and
|
||||
`TST-002` records the missing persistence seam. Stage 5 should check how
|
||||
status/restore presents abandoned runs; Stage 13 should decide whether true
|
||||
process interruption is accepted risk after handled errors are fixed.
|
||||
status/restore presents abandoned runs. The final accepted-risk section
|
||||
permits genuinely abrupt interruption residue only after handled errors are
|
||||
terminalized and session progress remains authoritative.
|
||||
|
||||
### `RSK-002`: single-file atomic replacement is not crash-durable
|
||||
|
||||
@@ -2502,8 +2687,9 @@ credible workload large enough to justify a performance finding.
|
||||
promoted-bundle paths; document platform/ACL limitations instead of assuming
|
||||
POSIX bits are universal.
|
||||
- Dependencies: Stage 6 confirmed there is no mode/config override or documented
|
||||
deployment privacy guarantee. Stage 7 owns adapter diagnostics, and Stage 13
|
||||
should rank the risk against deployment assumptions.
|
||||
deployment privacy guarantee. Stage 7 owns adapter diagnostics; the final
|
||||
backlog places secure defaults in the first safety workstream while retaining
|
||||
deployment ACL/umask details as an explicit evidence limitation.
|
||||
|
||||
### `RSK-005`: remote publish locks are race-prone snapshots
|
||||
|
||||
@@ -3002,14 +3188,12 @@ credible workload large enough to justify a performance finding.
|
||||
the later `ARC-002`/maintainability synthesis; this finding owns transport
|
||||
selection and operator truthfulness, not stage-state mechanics.
|
||||
|
||||
## Candidate Register
|
||||
## Classified Structural, Test, And Clarity Register
|
||||
|
||||
The `TST` entries now carry their Stage 12 classifications. Structural entries
|
||||
retain their original locations for traceability, but their Stage 11
|
||||
classifications are also final for this investigative pass: confirmed
|
||||
recommendations remain for Stage 13 prioritization, merged entries defer to the
|
||||
named stronger root, and rejected entries require new evidence before
|
||||
reopening.
|
||||
The entries retain their original locations for traceability, but their Stage
|
||||
11 or Stage 12 classifications are final: confirmed recommendations are ordered
|
||||
in the remediation backlog above, merged entries defer to the named stronger
|
||||
root, and rejected entries require new evidence before reopening.
|
||||
|
||||
### `ARC-001`: `IODecl` is not a complete or consistently classified stage contract
|
||||
|
||||
@@ -3921,7 +4105,7 @@ reopening.
|
||||
| --- | --- | --- |
|
||||
| Graph rollups `stage -> app`, `adapters -> app`, `config -> app` | rejected as a production reversal at Stage 1 | `go list` production imports contain no lower-level import of `internal/app`; graph connections include tests and ambiguous package grouping. Reopen only with a concrete production edge. |
|
||||
| Similar wrapper/manifest/adapter functions | rejected as broad abstractions at Stage 11 | Thin command wrappers, typed manifest models, distinct protocol constructors, and deliberately simple fakes share syntax rather than policy. Atomic byte writing is the narrow shared mechanism retained in `DUP-001`/`DUP-005`. |
|
||||
| Coverage percentages | deferred diagnostic signals, not findings | Stage 12 must reason from risk and test ownership, not a numeric target. |
|
||||
| Coverage percentages | rejected as standalone findings | Stage 12 used them only to revisit consequential branches and high-coverage duplication; the final test assessment sets no numeric target. |
|
||||
| Production fan-in/fan-out leaders | rejected as standalone Stage 11 findings | Stable owners such as configuration load/validate, canonical path helpers, manifest transitions, subprocess launch, and command dispatch should have many callers. Ambiguous method names and interface dispatch inflate graph rollups; caller tracing found no new ownership inversion. |
|
||||
| `previouscache.BuildPlan` complexity 22/38 | rejected as a simplification finding at Stage 11 | Its visible branches preserve required/optional absence, remote-current validation, candidate existence, and deterministic ordering. A helper would have no narrower policy owner; remote transfer cost is instead measured under `EFF-001`. |
|
||||
| Direct scan/allocation-in-loop graph signals | rejected as a production efficiency source at Stage 11 | Production filtering found no direct flagged occurrence. High transitive depth was composition/test propagation, and Stage 10 already rejected tiny artifact-set sorting as immaterial. |
|
||||
@@ -3984,69 +4168,82 @@ reopening.
|
||||
| Input `artifact` and `path` are adapter passthrough fields | confirmed as `COR-024` | Strict config accepts and documents them, but no production read, request field, CLI argument, or generated invocation field exists; values are silently discarded. |
|
||||
| Repeated analyze resolution branches justify a generic resolver framework | rejected; narrow `SIM-003` confirmed at Stage 11 | Source families deliberately differ in authority, provenance, optional absence, and repair guidance. A typed context/result and indexed effective plan reduce width without erasing those policies. |
|
||||
|
||||
## Unresolved Questions And Follow-Up
|
||||
## Open Decisions, Accepted Risks, And Limitations
|
||||
|
||||
- Should startup/status reconcile abandoned `running` invocation records, mark
|
||||
them interrupted, or retain them as an explicitly accepted audit limitation?
|
||||
- Which manifest model should retain run-scoped identity after `COR-001` is
|
||||
repaired, given that publish archives and automatic cleanup both consume it?
|
||||
- Which cross-platform root-relative filesystem primitive should own the
|
||||
symlink-safe mutation capability required by `COR-003` without moving stage
|
||||
policy into low-level `fileops`?
|
||||
- Should sentinel locking be replaced by an OS-released lock, or can a
|
||||
conservative ownership/recovery protocol satisfy `RSK-003` across supported
|
||||
platforms?
|
||||
- Which existing identifier spellings constrain the strict segment validation
|
||||
required by `COR-002`?
|
||||
- Do deployment packaging and service definitions already enforce an umask,
|
||||
ownership, or ACL that reduces `RSK-004`, and where should that contract be
|
||||
documented and tested?
|
||||
- What exact Scriptorium wire semantics, if any, were intended for input-level
|
||||
`artifact` and `path`? Until `COR-024` is answered, non-empty values must not
|
||||
continue to be accepted as meaningful passthrough configuration.
|
||||
- Should remote lock activation be snapshot-based or immediate, and what
|
||||
conditional-write capability can enforce the chosen rule across hosts?
|
||||
- Should restore consume only objects declared by the pointer-selected manifest,
|
||||
or should the committed model carry an immutable generation map that can bind
|
||||
every downloaded artifact, audio object, and previous-cache source?
|
||||
- Should restore's local commit point use a staging tree/snapshot swap, or can a
|
||||
journal plus locked revalidation make incremental replacement and recovery
|
||||
sufficiently explicit under `RSK-006`?
|
||||
- What durable identity should govern audio cache and existing-file reuse:
|
||||
storage version, ETag with provider-specific rules, or a published checksum?
|
||||
- Should restored manifests be rewritten to canonical local paths, or should all
|
||||
consumers ignore persisted absolute paths in favor of workspace-relative
|
||||
identities?
|
||||
- How should publish metadata retain a stable source-to-destination mapping for
|
||||
previous artifacts without reintroducing mutable configuration authority?
|
||||
- Should `--previous-session-id` remain a strict expectation or become an
|
||||
explicit override, and which compatibility cases already depend on omission?
|
||||
- Is any storage backend other than explicit S3 or disabled/local behavior part
|
||||
of the supported configuration language?
|
||||
- What ownership and maximum-size guarantees do deployment packages establish
|
||||
for the filesystem secrets directory, and can all supported platforms enforce
|
||||
no-follow regular-file reads?
|
||||
- Should remote session loading retain bytes until canonical preparation or
|
||||
expose an owned temporary handle whose cleanup spans every command consumer?
|
||||
- Is notification delivery a product requirement, and if so which provider
|
||||
protocol should become the canonical integration contract? Until that is
|
||||
answered, `ARC-004` requires non-placeholder settings to fail explicitly.
|
||||
- Which supported-platform process-tree primitive can give `RSK-011` equivalent
|
||||
termination and wait semantics on Linux, macOS, and Windows?
|
||||
- Which per-contract maximum sizes are operationally valid for Seriatim,
|
||||
Audita, Scriptorium, and bounds outputs under `RSK-013`?
|
||||
- Can subprocess environments be reduced to explicit allowlists, or must
|
||||
`RSK-012` retain broad inheritance and stream-redact every known sensitive
|
||||
value?
|
||||
- Should Notarius expose or Narratio require an explicit dependency-generation
|
||||
digest that covers profiles, prompts, modules, references, executable build,
|
||||
and other transitive inputs, or is documented operator-forced invalidation
|
||||
the intended long-term contract?
|
||||
- Which automation service and cadence should carry the validation required by
|
||||
`TST-012`, particularly the roughly 54-second race run, and should tagged
|
||||
release builds consume an already validated revision or repeat validation?
|
||||
The audit establishes the unsafe current behavior and smallest safe interim
|
||||
boundary; it does not make product or platform choices that require deployment
|
||||
knowledge. These decisions are inputs to the remediation sequence, not reasons
|
||||
to leave the current defects implicit.
|
||||
|
||||
No accepted risks or final audit conclusions are recorded yet.
|
||||
| Decision family | Owner | Required safe interim boundary | Related findings |
|
||||
| --- | --- | --- | --- |
|
||||
| Run-scoped manifest identity and abandoned invocation presentation | App/manifest maintainers | Recompute or validate one identity unit; terminalize every handled error. Status may label genuinely abandoned records without changing session progress authority. | `COR-001`, `RSK-001`, `SIM-001` |
|
||||
| Cross-platform confined mutation, durable replacement, and local locking | Fileops/artifacts owners with supported-platform maintainers | No path-based destructive operation may follow an untrusted ancestor; unsupported safe primitives must fail closed. Lock release failure must be observable. | `COR-003`, `RSK-002`, `RSK-003` |
|
||||
| Identifier compatibility | Config/artifacts owners with operators | Inventory deployed spellings before enforcing one opaque-segment grammar; meanwhile no constructor may return an escaped namespace. | `COR-002` |
|
||||
| Runtime permissions and filesystem-secret directory guarantees | Packaging/operations owner with app/fileops | Use private defaults and no-follow, bounded regular-file acquisition unless a documented service ownership/ACL contract proves a stronger equivalent. | `RSK-004`, `RSK-010` |
|
||||
| Scriptorium `artifact`/`path` semantics | Product owner and Scriptorium integration owner | Reject non-empty unsupported fields or remove them from the public contract until exact wire semantics are specified. | `COR-024` |
|
||||
| Remote commit representation, lock activation, and committed restore scope | Publish/restore/artifacts owners with storage capability owner | Preserve the old readable commit until one final atomic selection; restore and status must validate the same selected identity. Do not emulate conditional writes with an unsafe load/replace race. | `COR-004`, `COR-008`, `RSK-005`, `ARC-003` |
|
||||
| Restore local commit/recovery, content identity, canonical paths, and previous-source mapping | Restore/artifacts/audio/previouscache owners | Keep the manifest last, revalidate under the lock, never report unresolved conflicts as success, and prefer workspace-relative typed identity over foreign absolute paths or size-only reuse. | `COR-009` through `COR-011`, `RSK-006` through `RSK-008` |
|
||||
| Previous-session expectation and storage backend language | CLI/config/product owner | Preserve the documented flag as a strict expectation unless a compatibility review deliberately changes it; accept only explicitly implemented backend values. | `COR-014`, `COR-015` |
|
||||
| Remote session configuration lifetime | App configuration owner | Return an owned temporary handle/lifetime whose cleanup occurs after all command consumers finish; never persist ephemeral provenance as a durable path. | `RSK-009` |
|
||||
| Notification product requirement | Product and integration owner | Reject or clearly reserve non-placeholder settings until a provider contract and production sender exist. A no-op must be an explicit operator choice. | `ARC-004` |
|
||||
| Process trees, diagnostic redaction, and output size limits | Shared subprocess/adapter owners with platform and operations input | Kill and wait the owned tree, never persist known secret values, and use bounded no-follow regular-file acquisition. Choose limits per external contract rather than one global constant. | `RSK-011` through `RSK-013` |
|
||||
| External Notarius dependency identity | Notarius integration and operations owners | Continue to require/document `--force` for same-path external/transitive changes unless a version/digest contract is introduced; direct Narratio input bytes must still join the fingerprint. | `COR-020` |
|
||||
| Validation automation and race cadence | Repository maintainers | Every change and release revision must receive normal test/vet/build validation. Add race automation after `TST-001`; choose per-change versus scheduled cadence from measured capacity, not by omitting the check entirely. | `TST-001`, `TST-012` |
|
||||
|
||||
### Intentionally accepted risks
|
||||
|
||||
- The two manifest files do not form one atomic transaction. This is accepted
|
||||
because the session manifest is the sole progress authority and a run
|
||||
manifest is an invocation audit record. Remediation must test every durable
|
||||
disagreement boundary and terminalize handled errors; it need not introduce
|
||||
a distributed transaction.
|
||||
- An uncatchable process or host death may leave an old invocation record
|
||||
`running`. After handled errors are terminalized, that residual historical
|
||||
inaccuracy is accepted provided startup/status can distinguish or document it
|
||||
and the session manifest continues to drive conservative retry. It is not
|
||||
acceptable for controlled errors to leave the same ambiguity.
|
||||
- Restore remains an incremental operation without rollback. The cost and
|
||||
portability of a full workspace transaction are not justified by current
|
||||
evidence. This acceptance does not waive `RSK-006`: planning must be
|
||||
revalidated under the lock, partial state must be diagnosable/retryable, and
|
||||
the old manifest must not silently authorize replaced bytes.
|
||||
- Same-path changes to external Notarius executables, profiles, prompts,
|
||||
modules, references, environment, and provider behavior remain an
|
||||
operator-forced invalidation boundary. Narratio cannot infer all transitive
|
||||
external state without a new integration digest contract. Its direct trimmed
|
||||
transcript is not part of this acceptance and remains `COR-020`.
|
||||
- A configured artifact that is deliberately non-executable may be reused from
|
||||
its canonical non-empty file without prior analyze-manifest provenance.
|
||||
Freshness is operator-owned for that workflow. Explicit selection and
|
||||
prerequisite planning must still use one effective-set authority under
|
||||
`ARC-007`/`COR-022`.
|
||||
- The normal test suite remains serial where process-global fixtures exist; no
|
||||
blanket `t.Parallel` conversion is justified by its roughly 3.5-second
|
||||
runtime. Repeated shuffled execution may be scheduled rather than run on
|
||||
every change, provided `TST-011` is fixed and ordinary CI remains mandatory.
|
||||
|
||||
### Evidence limitations
|
||||
|
||||
- Dynamic validation ran on Linux/amd64 with Go 1.26.5. macOS and Windows were
|
||||
cross-build/reasoning targets only; platform-specific no-follow, directory
|
||||
sync, process-tree, and lock choices require focused native validation.
|
||||
- No live S3 service, paid API, production subprocess, credentialed operation,
|
||||
deliberate power loss, or destructive external workflow was exercised. The
|
||||
audit used source reasoning, existing deterministic fakes, temporary
|
||||
filesystems, loopback HTTP, and helper subprocesses as required by policy.
|
||||
- Crash durability, upload accepted-with-error behavior, remote conditional
|
||||
writes, and deployment ACL/umask protection remain environment-dependent.
|
||||
Their absence was confirmed in the current contracts, but exact provider and
|
||||
filesystem failure rates were not estimated.
|
||||
- The moderate code graph excludes documentation, examples, and the command
|
||||
entry point. Those artifacts were reviewed directly with repository tools;
|
||||
graph metrics were never used alone to confirm a finding.
|
||||
- This report is pinned to the implementation revision in the audit identity.
|
||||
Later commits through Stage 13 alter audit documentation only. Any production,
|
||||
test, canonical-contract, example, or dependency change invalidates the
|
||||
affected evidence and must trigger focused re-audit before remediation uses
|
||||
the conclusions.
|
||||
|
||||
## Completed-Stage Evidence
|
||||
|
||||
@@ -4577,3 +4774,43 @@ No accepted risks or final audit conclusions are recorded yet.
|
||||
remediation ordering, accepted-risk decisions, and final audit closeout. It
|
||||
should not reopen the risk-based test ownership and marginal-value decisions
|
||||
recorded here without new evidence.
|
||||
|
||||
### Stage 13
|
||||
|
||||
- Entry revision: `14ef59a` (`Document test suite policy audit conclusions`),
|
||||
with a clean worktree. The implementation, tests, examples, dependencies, and
|
||||
canonical current-behavior documents are byte-identical to the pinned audit
|
||||
revision; commits since it add only the audit plan, sequence, and ledger.
|
||||
- Scope and policy reviewed: development guide, all architecture,
|
||||
documentation, and testing policy, exact Stage 13 sequence, audit completion
|
||||
criteria, every lifecycle/scenario/risk matrix, all 81 stable finding IDs and
|
||||
their detailed evidence, the classification log, unresolved questions, and
|
||||
completed-stage records.
|
||||
- Revalidation evidence: current graph architecture and targeted ownership
|
||||
searches covered the runner/manifest, path/fileops, publish/current-state,
|
||||
restore/previous/audio, configuration/composition, adapters, ordinary stages,
|
||||
extraction, analyze/artifact, and focused-test boundaries. A repository diff
|
||||
from the pinned revision confirmed no affected source, test, contract,
|
||||
example, or module change. The prior exact snippets, callers, tests, and
|
||||
canonical contracts therefore remain current; no metric-only finding was
|
||||
admitted and no confirmed root required downgrade.
|
||||
- Reconciliation: 77 IDs remain confirmed. `ARC-002` and `COM-004` merge into
|
||||
`COM-002`; `SIM-002` merges into `DUP-007`; `DUP-004` remains rejected;
|
||||
`COM-005` remains independently confirmed only for its source/catalog
|
||||
omission while its no-output wording is owned by `COM-002`. Rejected signal
|
||||
themes remain in the classification log to prevent rediscovery.
|
||||
- Final report additions: executive assessment; completed scenario and audit-
|
||||
criteria matrices; positive conclusions; separate impact, likelihood,
|
||||
confidence, and scope ratings for structural/test/clarity items; an
|
||||
11-workstream dependency/risk-ordered remediation backlog; owned open
|
||||
decisions; intentionally accepted residual risks; and environmental/evidence
|
||||
limitations.
|
||||
- Validation: all relative links in `audit-findings.md`, `audit-plan.md`, and
|
||||
`audit-sequence.md` resolve; `git diff --check` passes. The full Stage 0
|
||||
implementation baseline was not rerun because Stage 13 proved that no
|
||||
implementation, test, dependency, example, or canonical contract changed;
|
||||
Stage 12's final normal suite and vet results remain the latest validation.
|
||||
- Exit conclusion: every audit-plan completion criterion and Stage 13
|
||||
deliverable is satisfied. No production, test, example, dependency, or
|
||||
current-behavior documentation change is included. The report is sufficient
|
||||
to prepare remediation work without repeating discovery.
|
||||
|
||||
Reference in New Issue
Block a user