# Feature 0001 Remediation Findings Status: resolved ## Review Scope This review compared `ansible-role-seriatim` commits `ac35c1f` through `efd4734` with the Feature 0001 roadmap and staged implementation plan. It also ran the committed static and integration validation and performed targeted state-corruption, permission-drift, and custom-path checks on the managed local installation. The four planned implementation stages are structurally present and the committed integration suite passes. The findings below are gaps that must be addressed before Feature 0001 is marked complete or its patterns are copied to other application roles. ## Finding 1: Build-input state content is not part of convergence Severity: high `tasks/build.yml` checks whether `build-inputs.json` exists but never reads or compares its content. The rebuild decision trusts `fingerprint.sha256` independently. An empty, corrupted, stale, or replaced build-input document is therefore accepted as converged when the fingerprint and binary checksum files remain unchanged. This diverges from the plan's requirement that malformed or unreadable state cause a rebuild and from the role's claim that readable inputs and their fingerprint describe the installed result. A targeted test truncated `build-inputs.json`; convergence returned `changed=0` and `failed=0`, while the subsequent verification play correctly failed its input/fingerprint assertion. Required remediation: - Read the stored build-input document safely during convergence. - Require its exact canonical content to match the desired build inputs and require its SHA-256 digest to match the stored and desired fingerprint. - Treat missing, unreadable, malformed, or inconsistent input state as a rebuild condition rather than an unhandled failure. - Extend `tests/integration` to corrupt or replace `build-inputs.json`, verify that convergence repairs it, and verify that the following run is idempotent. ## Finding 2: Managed ownership and modes drift without repair Severity: medium The binary's checksum and version participate in the rebuild decision, but its owner, group, and mode do not. Binary metadata is set only inside the rebuild block. Build-state directory and file metadata are likewise set only after a build. A content-identical installation can therefore remain in an unsafe or incorrect metadata state indefinitely. A targeted test changed `/usr/local/bin/seriatim` from mode `0755` to `0777`. Convergence returned `changed=0` and left mode `0777` in place. This violates the target state requiring a root-owned, non-operator-writable binary. The same design leaves drifted state-file ownership and permissions uncorrected. Required remediation: - Converge `seriatim_binary_path` to `root:root` mode `0755` whenever a valid installed regular file exists, without forcing an unnecessary rebuild. - Converge `seriatim_build_state_dir` to `root:root` mode `0755` and its three managed files to `root:root` mode `0644` on every successful role run. - Treat unexpected file types, including a directory at the binary path, as a clear validation or repair condition rather than passing them into the build copy operation. - Add integration cases that drift binary and build-state metadata, require one repairing run, and require an idempotent repeat. ## Finding 3: A configurable binary path requires a pre-existing parent Severity: medium `seriatim_binary_path` is a documented public path override, but the role does not create or validate its parent directory. `ansible.builtin.copy` consequently fails after a successful build when the configured parent is absent. A targeted run using `/tmp/seriatim-review-missing-parent/seriatim` failed with `Destination directory ... does not exist`. Required remediation: - Inspect the configured binary parent before building. - Fail clearly if an existing parent is not a directory. - Create a missing parent as `root:root` mode `0755` without changing ownership or mode on an already existing shared directory. - Add an isolated integration case proving that a nondefault binary path with a missing parent installs, validates, and converges idempotently. ## Finding 4: Role metadata and privilege documentation are incomplete Severity: low Stage 1 required role source and issue information, but `meta/main.yml` omits the supported `issue_tracker_url` field. The README also does not state plainly that consumers must apply the role with privilege escalation, even though it manages packages, snaps, root-owned source, and `/usr/local/bin`. Required remediation: - Add the role repository's Gitea issue tracker to `meta/main.yml`. - Add role repository and issue links to the README where appropriate. - State explicitly that the consuming play must use `become: true` or provide equivalent root privileges. - Re-run metadata/schema validation and ansible-lint. ## Remediation Completion Criteria - All four findings are addressed in `ansible-role-seriatim`. - Static validation and the expanded `tests/integration` suite pass. - Targeted corruption, metadata-drift, and alternate-path cases converge to the documented state and then report `changed=0` on an immediate repeat. - The default local installation ends at Seriatim `v1.5.0`, source commit `0fc92f364316c874d366b00cdc884a33e1900b25`, with a clean checkout and consistent build state. - The role and workspace repositories are clean, and remediation commits are ready for review before Feature 0001 is marked complete. ## Resolution Evidence - Finding 1 was resolved by `ansible-role-seriatim` commit `c6c1907`. - Finding 2 was resolved by `ansible-role-seriatim` commit `f2c1b1a`. - Finding 3 was resolved by `ansible-role-seriatim` commit `92906eb`. - Finding 4 was resolved by `ansible-role-seriatim` commit `07dd736`. - Malformed state-object recovery was hardened by `ansible-role-seriatim` commit `ca49dae` during the completion audit. - The expanded `tests/integration` suite passed after all remediation commits and restored the default Seriatim installation.