diff --git a/docs/roadmap/potential-audit.md b/docs/roadmap/potential-audit.md new file mode 100644 index 0000000..dc8070f --- /dev/null +++ b/docs/roadmap/potential-audit.md @@ -0,0 +1,224 @@ +# Potential Maintainability Audit + +## Status + +Initial impressions only. This document does not define action items, commit to +an audit, or recommend specific refactoring. Its purpose is to preserve signals +that may warrant investigation in a subsequent, deeper pass. + +## Context + +Narratio contains approximately 36,000 lines of production Go and 45,000 lines +of test Go across 26 packages. That footprint appears broadly consistent with a +durable workflow orchestrator that owns configuration, pipeline sequencing, +artifact and manifest state, recovery, storage, subprocess integration, and +filesystem safety. It is larger than would be expected of a simple command +wrapper, but the current size alone is not evidence of a maintainability +problem. + +Narratio handles non-sensitive pipeline data and is expected to run in a +trusted environment. A future review therefore need not assume an adversary who +can manipulate workspace paths, symlinks, or directory entries concurrently +with the process. Protection against accidental path escape and destructive +operator mistakes remains useful, as do atomic replacement, crash durability, +and recoverable state transitions. The distinction between adversarial +filesystem hardening and crash-safe data integrity should guide any proposed +simplification. + +## Preliminary Signals + +The following observations may justify a focused maintainability audit: + +- Production code is concentrated in `internal/stage`, `internal/app`, and + `internal/config`. These packages contain approximately 8,300, 7,900, and + 5,500 lines respectively. +- Several production files exceed 1,000 lines, including stage publishing and + analysis, configuration validation, and application runner code. File size + does not establish poor cohesion, but it can conceal multiple responsibilities + or make change impact harder to assess. +- The application, stage, and artifact areas have substantial interaction. + Their effective boundaries and the blast radius of changes may merit closer + examination. +- The most notable function-level complexity observed so far is concentrated in + confined filesystem and directory-promotion code. This is security-sensitive + logic built around a stronger threat model than Narratio now requires. A + review could determine which mechanisms exist only to resist adversarial + filesystem races and which are necessary for path safety, atomic updates, and + crash-safe integrity. +- Test code is larger than production code. This is a positive verification + signal, but a deeper pass could determine whether the suite also contains + repetition, brittle implementation coupling, or fixture and helper sprawl. + +## Counter-Signals + +The initial review also found evidence against a conclusion of broad or urgent +maintainability trouble: + +- Repository-wide tests and `go vet` pass. +- Package dependencies are acyclic. +- Average function complexity is modest, with higher complexity appearing + localized rather than systemic. +- The direct dependency set is small. +- Documentation is structured around explicit ownership and reading paths. +- The repository has substantial automated test coverage by volume. + +## Questions for a Deeper Pass + +A later audit could investigate, without presuming that changes are required: + +- whether responsibilities within `internal/app`, `internal/stage`, and + `internal/config` remain cohesive; +- whether the largest files contain natural internal modules that are currently + obscured; +- whether stage and external-adapter implementations contain meaningful + duplication; +- whether tests are organized around durable behavior or coupled to incidental + implementation structure; and +- whether manifest, restore, publishing, and confined-filesystem invariants are + easy to locate, understand, and verify across package boundaries. + +The deeper pass should also evaluate these specific modernization candidates: + +- upgrading the maintained toolchain baseline from Go 1.25 to Go 1.27, subject + to ordinary compatibility and release validation; +- replacing custom root-confinement, no-follow traversal, safe cleanup, and + confined-read machinery with the standard library's `os.Root` operations + where their semantics match Narratio's reduced threat model; +- retaining Narratio-owned code where it still provides application-specific + crash durability, atomic replacement, no-replace directory promotion, mode + policy, bounded reads, or destructive-operation guardrails; and +- evaluating the `os.Process.WithHandle` API added in Go 1.26 as a way to avoid + reopening a Windows child process by PID before assigning it to an owned job + object, while recognizing that it may not replace suspended startup, job + object management, or initial-thread resumption; +- using the standard library's `slices` and `maps` packages to replace generic + cloning, membership, key extraction, sorting, and deduplication helpers where + those helpers do not encode domain-specific ordering or copy semantics; +- using `golang.org/x/sync/errgroup`, including its bounded-concurrency support, + to simplify transcription worker coordination, cancellation, and first-error + propagation while retaining Narratio-owned result aggregation and error + context; +- adopting `github.com/google/go-cmp/cmp` selectively in tests where structural + diffs would replace `reflect.DeepEqual` or hand-written comparisons with more + useful failure output; +- evaluating `github.com/gofrs/flock` after the `os.Root` design is understood, + to determine whether it can replace Unix and Windows lock primitives and + context-aware polling without losing lock metadata or desired file ownership + semantics; and +- considering a CLI framework such as Cobra only if command and flag evolution + proves costly enough to justify migration risk, dependency growth, and + possible changes to existing parsing, help, and error-output behavior. + +The review may also document why a replacement is not appropriate. Current +low-confidence candidates include general configuration frameworks or schema +validators, generic atomic-file packages, provider-neutral object-storage +abstractions, retry or backoff packages, UUID or ULID packages, and generic +subprocess frameworks. Narratio's composition provenance, semantic validation, +durability guarantees, conditional object writes, identifier needs, and +tool-specific adapter contracts may leave little generic machinery for these +libraries to replace. + +## Preliminary Complexity Findings + +Complexity metrics are heuristics rather than findings of defects. Guard +clauses, validation branches, and explicit state transitions can score highly +while remaining appropriate. The following production-only measurements are +initial signals for focused code reading. + +### Execution And Planning + +`internal/app.executePlan` is the clearest outlier observed so far: + +- approximately 456 lines; +- cyclomatic complexity of 79; +- cognitive complexity of 149; and +- more than 60 direct collaborators in the code graph. + +The function currently spans invocation preparation, collaborator construction, +locking, session and run manifest initialization, resume decisions, stage +transitions, invalidation, terminal-failure persistence, and post-publish +cleanup. A deeper pass should look for existing lifecycle boundaries that could +be made explicit without redesigning the state machine during extraction. + +`internal/app.Plan` is approximately 110 lines with cyclomatic complexity 27 +and cognitive complexity 51. More importantly, it independently models parts +of the run/skip, resume-validation, stale-marking, and invalidation behavior +also present in execution. The audit should examine whether a shared, pure stage +transition model could reduce drift between planning and execution. + +### Analyze Orchestration + +The analyze implementation has two notable orchestration functions: + +- `analyzeStage.Run` is approximately 252 lines with cyclomatic complexity 33 + and cognitive complexity 47; and +- `executeAnalyzeArtifact` is approximately 259 lines with cyclomatic + complexity 28 and cognitive complexity 39. + +`internal/stage/analyze.go` combines setup, catalog construction, +reconciliation, planning, per-artifact execution, state projection, dependency +invalidation, reuse aggregation, evidence registration, and result assembly. +Possible seams to investigate include execution-state initialization, +current/reused record projection, a per-artifact transaction, evidence +registration, and final result accumulation. + +### Configuration Validation And Composition + +`internal/config/validate.go` contains approximately 1,100 lines and has the +highest aggregate file complexity observed: cyclomatic complexity 264 and +cognitive complexity 394 across its functions. Notable functions include: + +- `validateScriptorium`: approximately 117 lines, cyclomatic complexity 32, + cognitive complexity 74; +- `validateNotarius`: approximately 129 lines, cyclomatic complexity 29, + cognitive complexity 42; and +- `validateAudita`: approximately 54 lines, cyclomatic complexity 21, + cognitive complexity 34. + +These scores are partly driven by explicit validation guards. A deeper review +should determine whether artifact-level, dependency-graph, path/output, +input-source, and variable validation can be separated while preserving precise +field-specific errors. + +Composition complexity is concentrated in `internal/config/composition.go` and +`internal/config/pipeline_composition.go`. In particular, +`splitPipelineCompositionEnvelope` is approximately 87 lines with cyclomatic +complexity 21 and cognitive complexity 63. It combines YAML shape checking, +source-aware diagnostics, profile parsing, default-profile validation, cloning, +and removal of composition metadata. Structural decoding and semantic +validation may be separable, but the audit should preserve source provenance +and field ownership. + +### Previous-Session Cache + +`internal/previouscache.Resolve` is approximately 180 lines with cyclomatic +complexity 29 and cognitive complexity 60, within a package of only about 483 +production lines. It combines configuration checks, current-state loading, +required and optional behavior, immutable-commit resolution, legacy candidate +probing, local-path construction, record creation, and sorting. Modern +immutable-commit resolution and legacy probing are distinct algorithms and may +form a useful extraction boundary. + +### Additional Areas To Inspect + +Lower-priority complexity concentrations include: + +- `internal/stage/publish.go`, approximately 1,175 lines with aggregate + cognitive complexity 239, where file collection, output resolution, + publication, and commit assembly may be separable; +- `internal/fileops/directory.go`, approximately 555 lines with aggregate + cognitive complexity 184, some of which may be removed by the `os.Root` + evaluation; +- `internal/app/restore_plan.go`, particularly the approximately 129-line + `classifyRestoreAction`; and +- `internal/manifest/analyze_state.go`, whose moderately complex validation may + be justified by its ownership of explicit state invariants. + +On the initial evidence, execution and shared plan/execution transition logic +should receive the first detailed review. Configuration and analyze +orchestration form the next tier, while their higher complexity may more often +reflect detailed domain rules. + +These questions should be revisited with change-history analysis, focused code +reading, and maintenance experience before forming recommendations or creating +work items.