4.5 KiB
Notarius Cache Reuse Review Remediation
Status: Complete
Review scope: commits 5086596 through a0312ed, compared with the accepted
cache roadmap and implementation plan. The core cache integration works,
including the real Notarius v0.7.0 integration test. Address the following
bounded findings; no broader cache redesign is required.
1. P2 — Preserve Relative Workspace Support at the Snapshot Boundary
Status: Complete. Snapshot materialization and post-run verification normalize their local session/cache roots to absolute paths. Regression coverage proves stable absolute adapter bindings, successful production-adapter launch, absence of the duplicated directory tree, and rejection of a symlink on the actual cache path for a relative workspace.
Location: internal/stage/extract_references.go, especially
extractReferenceSnapshotDestination and checkExtractSnapshotDirectory;
internal/artifacts/paths.go snapshot path construction.
The new snapshot destination inherits SessionPaths.CacheDir verbatim. A
relative workspace root therefore produces a relative reference binding,
whereas the previous extract implementation explicitly converted its snapshot
root to an absolute path. The Notarius adapter requires absolute reference
paths and rejects the new binding before launch. Configuration does not require
an absolute workspace root.
There is a related path-composition defect: checkExtractSnapshotDirectory
passes an already session-prefixed relative destination to
pathsafe.SlashRelativeFromRoot, which interprets relative targets as relative
to the supplied root. The resulting directory walk duplicates the session
prefix instead of checking the actual destination ancestry. Subsequent copying
can create the intended directories through a different path, leaving the
wrong extra directory tree and bypassing the intended component checks there.
Evidence: a temporary focused test created a valid prepared party reference,
expressed the session/cache roots relative to the working directory, and called
the production snapshot materializer. It returned a binding beginning
work/campaign-a/.../cache/notarius/references/.../party.yml, not an absolute
path. The diagnostic test was removed after the review.
Required change:
- Normalize the session root and snapshot destination consistently to absolute paths before filesystem checks and adapter delivery, preserving the existing meaning of relative workspace configuration.
- Ensure the component walk checks the same path subsequently read/written.
- Do not fix this by requiring operators to configure absolute workspace roots or by changing global path semantics unnecessarily.
- Add a regression using a relative workspace, real snapshot materialization, and the adapter validation boundary. Verify absolute stable bindings across invocations, no duplicated session-prefix directory, and rejection of an unsafe component on the actual cache path.
2. P3 — Remove the Accidental Reference-Version Requirement
Status: Complete. The integration contract now distinguishes the v0.6.0 reference capability from selected cache flags and the tested v0.7.0 baseline, without introducing a runtime gate.
Location: docs/integrations/notarius.md, Invocation section, line 28.
The documentation now says Narratio requires the Notarius v0.7.0 CLI contract when references are configured. References were already supported under the documented v0.6.0 contract. The implementation plan selected v0.7.0 as the tested cache integration baseline and explicitly did not require rejecting older compatible executables. The revised sentence unnecessarily suggests an upgrade is mandatory for existing reference-only use.
Required change: distinguish the tested v0.7.0 cache integration baseline from the reference CLI contract. Document required capabilities for the selected flags without claiming a new unconditional minimum version. Keep runtime behavior unchanged; do not add a version probe or gate.
Validation Recorded
GOWORK=off go test ./...passed.- Race tests for stage, app, config, artifacts, and Notarius adapter passed.
GOWORK=off go vet ./...passed.- The opt-in
TestNotariusCacheReuseIntegrationpassed against the previously built local v0.7.0 binary with its local test provider, covering actual reuse, reference-change misses, partial recovery, and forced chunk refresh. - The additional relative-path diagnostic failed as described above; this case was not covered by the existing absolute-path fixtures.