Close Feature 0011 after remediation

This commit is contained in:
2026-09-16 21:37:17 +00:00
parent d2bd48cc99
commit 54a2362c02
5 changed files with 158 additions and 748 deletions

View File

@@ -1,112 +1,149 @@
# Feature 0010 Remediation Review
# Feature 0011 Remediation Review
Status: resolved
Reviewed implementation commits:
- `14a38e0 Upgrade the Notarius role to version 0.7.0`
- `90893ae Integrate Notarius extraction into the D&D pipeline`
- `7fd7b24 Document and test the D&D Notarius integration`
- `292d866 Upgrade the Narratio role to version 2.1.0`
- `28a4d27 Add managed credentials to the D&D pipeline`
- `8ff0761 Document and test pipeline credential delivery`
## Review Summary
Feature 0010 implements the intended production architecture. The standalone
Notarius role supports ConfigKit imports without owning imported fragments. The
pipeline role owns the complete D&D extraction topology, renders it as a
ConfigKit fragment, selects it from the root Notarius configuration, connects
Narratio to that pipeline and its artifact paths, and manages a shared FHS
cache with the pipeline's group-access policy. The complete standalone and
pipeline integration suites pass, including native application validation and
immediate clean repeats.
Feature 0011 implements the principal target architecture. The standalone
Narratio role now installs v2.1.0 and generically exposes its three native
Notarius controls. The pipeline pins that role revision, manages a restricted
conventional secrets directory, coordinates the Audita and Notarius credential
references, and enables ordinary Notarius checkpoint reuse without duplicating
the root-owned chunk-cache policy. Both complete integration entry points pass,
including native application validation and immediate clean repeats.
No production behavior gap was found. Two low-severity issues should be
addressed: the configurable pipeline-ID boundary is not observed through
Narratio in the integration suite, and the ACL task names no longer describe
all of the roots they manage.
Two issues remain. First, the feature roadmap and implementation plan
strengthened the requested credential-delivery interface with deletion and
empty-value policies that were not required. The implementation consequently
contains an extra production task and substantial test and documentation
machinery for those invented semantics. Second, the pipeline README does not
explain the Scriptorium credential flow that the feature roadmap expressly
requires it to document.
## Finding 1: Alternate Pipeline Selection Is Not Observed Through Narratio
## Finding 1: The Credential Mapping Adds Unrequested Lifecycle Semantics
Severity: low
Severity: moderate
`ansible-role-dnd-pipeline/tests/integration:172-187` composes two alternate
Notarius pipeline maps and selects `archive-session` through
`dnd_pipeline_narratio_notarius_pipeline_id`. The test then validates only the
Notarius configuration and cache-mode override. It does not inspect Narratio's
effective configuration after the alternate convergence.
The user requested an inventory-friendly way to deliver credential values to
Narratio's conventional secrets directory. The roadmap subsequently added two
stronger behaviors without an express requirement: null means delete an exact
file, and an empty string must be rejected by the role. Those choices were
then made mandatory by the implementation plan.
The production wiring in `ansible-role-dnd-pipeline/meta/main.yml:45-50` is
correct, but a future regression that stops forwarding the selected pipeline
ID to Narratio could pass the current test. This is a focused behavioral
coverage gap at a feature-owned integration boundary, not a request for a
snapshot or implementation-detail assertion.
The resulting production implementation appears in
`ansible-role-dnd-pipeline/tasks/main.yml:10-53`. It validates the stronger
value policy, conditionally copies non-null entries, and uses a separate file
loop to delete null entries. The public contract and example are repeated in
`ansible-role-dnd-pipeline/README.md:275-291`, while
`ansible-role-dnd-pipeline/tests/integration:167-194` adds an empty-value
failure case and a multi-convergence deletion lifecycle.
The environment-variable-name validation is justified because caller input
becomes a destination path. The two value semantics are different: they add
credential lifecycle and application-validity guarantees that the role was
not asked to own. ConfigKit accepts an empty immediate credential file and
performs its own nonempty check only when a configured typed reference resolves
to that value. A simple delivery role should write the caller-supplied string
and trust the native application boundary. Credential deletion can be added
later if an operator-facing removal contract is expressly requested.
### Required Remediation
- After the alternate convergence, use Narratio's native configuration command
with a deployed campaign and inspect the effective selected pipeline ID.
- Assert that Narratio observes `archive-session`.
- Retain the existing native Notarius checks, cache-mode check, and immediate
clean repeat.
- Do not run model-backed extraction or snapshot the complete generated
configuration.
- Narrow `dnd_pipeline_secrets` to an empty-by-default mapping from valid
environment-variable names to string file contents.
- Retain complete key validation before path construction and retain
`no_log: true` for every task that evaluates or writes credential values.
- Validate that values are strings, but do not add a nonempty policy that is
stricter than the native bootstrap and typed-reference boundary.
- Converge every supplied entry directly with one `ansible.builtin.copy` loop.
Remove the null condition and the separate deletion task.
- Treat omitted keys as unmanaged. Do not invent a replacement deletion,
purge, directory-mirroring, or credential-validation mechanism.
- Remove the empty-value and explicit-null lifecycle cases from the integration
suite. Retain the valid-name boundary case, representative credential
delivery, redaction checks, authorized native loading, and immediate clean
repeat.
- Correct the Feature 0011 roadmap, the settled MVP text, and the pipeline
README so they describe the narrower interface and no longer present the
strengthened semantics as user decisions.
## Finding 2: ACL Task Names Exclude the Notarius Cache They Manage
## Finding 2: Scriptorium Credential Delivery Is Not Documented for Consumers
Severity: low
`ansible-role-dnd-pipeline/tasks/main.yml:19` and
`ansible-role-dnd-pipeline/tasks/main.yml:39` describe their ACL loops as
managing "Narratio writable roots." Both loops now also manage the Notarius
cache root. The task behavior is correct, but the execution log is misleading
about its scope.
The Feature 0011 roadmap requires documentation of all application credential
flows, including Scriptorium's existing profile-owned `api_key_env` selection.
`ansible-role-dnd-pipeline/README.md:275-291` explains the generic mapping and
the earlier Notarius section explains its typed reference, but the README never
tells an operator how a selected Scriptorium profile obtains its credential.
Production behavior is already correct: Narratio bootstraps the named file into
its environment and its existing Scriptorium adapter forwards the selected
profile credential. This is a documentation gap, not a request for another
variable, task, or integration test.
### Required Remediation
- Rename both ACL tasks to describe the shared pipeline writable roots.
- Do not otherwise change the direct desired-state ACL implementation or its
task flow.
- Add a concise consumer-facing explanation that a Scriptorium profile remains
the owner of its `api_key_env` name.
- Explain that an operator can add a matching filename/value entry to
`dnd_pipeline_secrets`; Narratio's bootstrap supplies it and the existing
adapter forwards only the profile-selected credential.
- Do not add a suite-specific Scriptorium credential variable, inspect profile
files in Ansible, duplicate the application adapter, or add tests of
Narratio/Scriptorium internals.
## Resolution
Resolved in pipeline-role commit `bbf01e1`. The alternate-map integration case
now uses Narratio's native `config show` command to observe the selected
`archive-session` pipeline ID before its immediate clean repeat. The two ACL
task names now describe shared pipeline writable roots without changing their
module arguments, loops, or task flow.
Resolved in pipeline-role commit `de1b191`. The credential mapping now accepts
valid environment-variable names with string contents, validates the complete
mapping before path construction, and converges every supplied entry through
one secret-safe `ansible.builtin.copy` loop. The unrequested empty-value policy,
null deletion task, and associated lifecycle tests were removed. Omitted names
remain unmanaged.
The pipeline README now explains how a Scriptorium profile's application-owned
`api_key_env` name selects a matching `dnd_pipeline_secrets` entry without
adding another suite variable or duplicating adapter behavior. The durable
Feature 0011 roadmap and MVP policy were corrected to record the narrower
operator-facing contract.
## Required Validation
After remediation, run:
Remediation validation:
```text
cd ansible-role-dnd-pipeline && tests/integration
git diff --check
```
The expected result is successful native configuration validation for the
base, testing, and alternate profiles; observation of `archive-session` through
Narratio; successful shared-cache access; and immediate repeats with
`changed=0` and `failed=0`.
The result was successful syntax and lint validation; synthetic
credential delivery through the conventional directory; native Narratio
configuration loading for authorized identities; absence of synthetic values
from captured output; preservation of Notarius resume and root-owned `auto`
chunk caching; and immediate repeats with `changed=0` and `failed=0`.
## Review Validation Evidence
The independent review inspected the full implementation diffs and confirmed
that the dependency pins resolve to the intended role commits. A semantic
comparison found that the managed D&D pipeline topology matches the Notarius
v0.7.0 example after removing only the external reference paths that belong to
Narratio. The Narratio output map matches its upstream annotated example, and
the intentional omission of the optional spell catalog agrees with the feature
roadmap.
Both `ansible-role-notarius/tests/integration` and
`ansible-role-dnd-pipeline/tests/integration` passed. Post-suite checks also
confirmed the expected Seriatim and Notarius versions, native Notarius pipeline
validation, native Narratio validation for the base and testing profiles, and
a clean repeat. `git diff --check` passed, and all repositories were clean
before this review document was replaced.
The review inspected the exact implementation diffs and confirmed that the
Narratio dependency pin resolves to the reviewed `292d866` revision. The
standalone Narratio `tests/integration` entry point passed with v2.1.0 native
configuration validation and clean repeats. The complete pipeline
`tests/integration` entry point also passed, including isolated dependency
installation, syntax checking, production-profile lint, authorized and denied
native loading, redaction checks, profile and version selections, and clean
repeats. No live model, transcription, storage, or publishing endpoint was
contacted.
After remediation, the complete pipeline integration entry point passed again.
It observed `archive-session` through Narratio, validated the alternate
Notarius pipeline, and reported `changed=0` and `failed=0` on the identical
repeat. Cleanup restored Seriatim v1.5.0 and the default `dnd-session`
configuration. `git diff --check` also passed.
It exercised isolated dependency installation, the invalid-name boundary,
synthetic credential delivery and redaction, authorized native loading,
Notarius resume and root-owned `auto` chunk caching, the retained profile and
version selections, and immediate clean repeats. `git diff --check` also
passed before the remediation commit.