Problem
These came from Helper-02's second-session delta review of #16351 round 13 (3aadb1424..ae04398c2). Nothing here blocks #16351. All of them sharpen guards that pass non-vacuously on today's playbook but could pass vacuously after a plausible edit. They were filed rather than folded in because #16351 had reached thirteen rounds, and every push re-runs full CI while runners are saturated.
| # |
Where |
Gap |
Fix |
| A |
repo_tests/sync_deletions_cross_component_isolation_16310_test.py, _assert_no_archive_prefix_rewrite |
It reads only the task's shell key. Converting the archive task to ansible.builtin.shell: or command: makes the check pass with nothing checked. |
Read whichever command key is present (shell, ansible.builtin.shell, command, ansible.builtin.command, cmd), and assert that a command was found. |
| B |
same |
git's ref:path form (git archive <ref>:autobot-backend) flattens the paths without any --prefix, so the landing-directory derivation would be wrong while the prefix check still passes. |
Pin the -- {{ item.path }} pathspec form itself, and fail on a <ref>:<path> tree-ish. |
| C |
same, the strip-components branch |
Any --strip-components=N is taken to mean "dest is the landing directory". That's only right when N equals the archived path's depth: 4 for the AI Stack, correct today. |
Derive the landing directory from N and the archived path's depth, and fail if they don't add up. |
| D |
autobot-slm-backend/ansible/roles/_shared/tasks/sync_deletions.yml, the always: cleanup |
failed_when: false makes a cleanup failure silent. |
register: the result and add a debug warning when it failed, so "nothing swallowed" stays true. |
| E |
same, comment |
ignore_unreachable: true only covers a node that became unreachable between the delete and the cleanup. A node that's already unreachable inside the block isn't rescued, and it leaves the play before always:. The "one node never aborts the fleet" contract covers this module's failures, not unreachability. |
State that boundary in the comment. |
| F |
the isolation test's docstring |
The new isolation tests cover the 12 playbook sites. The four role sites (e.g. slm_agent's per-file copies) are outside them, because their targets are role variables. |
State the boundary, and extend coverage to the role sites if their variables can be resolved statically. |
Acceptance criteria
Refs #16351, #16310, #16322
Problem
These came from Helper-02's second-session delta review of #16351 round 13 (
3aadb1424..ae04398c2). Nothing here blocks #16351. All of them sharpen guards that pass non-vacuously on today's playbook but could pass vacuously after a plausible edit. They were filed rather than folded in because #16351 had reached thirteen rounds, and every push re-runs full CI while runners are saturated.repo_tests/sync_deletions_cross_component_isolation_16310_test.py,_assert_no_archive_prefix_rewriteshellkey. Converting the archive task toansible.builtin.shell:orcommand:makes the check pass with nothing checked.shell,ansible.builtin.shell,command,ansible.builtin.command,cmd), and assert that a command was found.ref:pathform (git archive <ref>:autobot-backend) flattens the paths without any--prefix, so the landing-directory derivation would be wrong while the prefix check still passes.-- {{ item.path }}pathspec form itself, and fail on a<ref>:<path>tree-ish.--strip-components=Nis taken to mean "destis the landing directory". That's only right when N equals the archived path's depth: 4 for the AI Stack, correct today.autobot-slm-backend/ansible/roles/_shared/tasks/sync_deletions.yml, thealways:cleanupfailed_when: falsemakes a cleanup failure silent.register:the result and add adebugwarning when it failed, so "nothing swallowed" stays true.ignore_unreachable: trueonly covers a node that became unreachable between the delete and the cleanup. A node that's already unreachable inside the block isn't rescued, and it leaves the play beforealways:. The "one node never aborts the fleet" contract covers this module's failures, not unreachability.Acceptance criteria
<ref>:<path>archive fails the test.Refs #16351, #16310, #16322