ADVERSARIAL REVIEW -- D-139 STEP 6 TOOL
=======================================
Date : 2026-08-02
Reviewer : adversarial review pass (subagent), jumphost vcloud
Targets : netbox/d139-step6-vip-rehome.py (untracked, written 2026-08-02)
tests/d139-step6-vip-rehome/run-tests.sh (untracked, written 2026-08-02)
Governing : docs/design-decisions.md
- "ORDERING RULING 2026-08-02 -- D-139 step 6 executes IN FULL before the
Stage-5 deploy" -- operator: "Full step 6 first, then deploy"
- "AMENDMENT 2026-08-02 -- D-139 step 6: 'retire' means DEPRECATE, and
nothing is deleted" -- operator: "Deprecate both, delete nothing"
- "CORRECTION NOTE 2026-08-01 -- D-139: the EXECUTION list was defective"
Apex : http://10.10.1.10:8000 (office1-netbox, the WORKING VR1 apex, DOCFIX-195)
VERDICT: FIX FIRST
Do NOT run --commit as the tool stands. Two MEASURED defects are reachable without any
operator error beyond choosing the other valid value of a required flag, and the harness
does not test the one property both operator rulings name as load-bearing.
The tool's core mapping logic is CORRECT and its dry run reproduces the ruling's
measured 26 / 26 / 9 exactly. The defects are missing PRECONDITIONS and missing
COVERAGE, not a wrong carve.
METHOD AND HONESTY BOUNDARY
MEASURED = observed from a command run this session, output reproduced below.
REASONED = derived from reading the source; explicitly labelled where used.
Nothing in the DEFECTS section is REASONED-only; every one carries a run.
--commit was NEVER executed. All live apex access was read-only (GET).
Mutation experiments were run against COPIES under
/home/jessea123/.claude/jobs/015c103e/tmp/mut/ . No repo file was edited at any point.
-------------------------------------------------------------------------------
BASELINE -- what the tool does today, MEASURED
-------------------------------------------------------------------------------
$ . ~/vr1-office1-creds/vr1-netbox-sandbox.env
$ python3 netbox/d139-step6-vip-rehome.py --dc vr1-dc0 (DRY RUN, rc=0)
D-139 STEP 6 -- vr1-dc0 -- apex http://10.10.1.10:8000
CREATE 26 | ALREADY 0 | DEPRECATE-ADDR 26 | DEPRECATE-PFX 9
The 26 creates are 2602:f3e2:f02:20::50-::62 (metal-admin) and
2602:f3e2:f02:21::50-::62 (metal-internal), status=reserved, descriptions verbatim.
This matches the ORDERING RULING's measured state exactly. Host octets are preserved,
the ULA->GUA hextet mapping is right, and the /60 parents are deprecated but hold no
addresses. The offline harness reports PASS=14 FAIL=0.
Independent read-only enumeration of the apex (152 prefixes, 160 addresses, 27 ranges)
confirms, MEASURED:
- 52 ULA v6 addresses exist: 26 in dc0 (fd50:...:220,:221), 26 in dc1 (:320,:321).
- All 26 dc0 ULA VIP records carry status=reserved. VIP_STATUS="reserved" is correct.
- Zero ip-ranges inside the retired ULA /48. Zero duplicate prefix CIDRs apex-wide.
- No provider-public ULA /64 exists, so the ambiguity REFUSE (:10 vs :11) is not
reachable on this apex today. It is a live-safe design, not a live-blocking one.
CHECKED AND CLEAN (a gap I suspected and disproved):
overlays/vr1-dc0-vips.yaml declares 39 GUA VIP legs -- f02:11 x13, f02:20 x13,
f02:21 x13 -- and ZERO fd50: legs. Step 6 only creates 26. The missing 13 are NOT a
gap: 2602:f3e2:f02:11::50-::62 ALREADY EXIST in the apex as ip-address records
("VIP <svc> provider-public v6 (vr1-dc0) -- octet mirror ::NN"). So 13 existing + 26
created = the full 39, and after step 6 the apex records every VIP the overlay
declares. No action needed.
-------------------------------------------------------------------------------
DEFECTS, RANKED BY CONSEQUENCE
-------------------------------------------------------------------------------
DEF-1 [CRITICAL -- defect MEASURED, consequence REASONED]
--dc vr1-dc1 creates 26 ORPHAN addresses and deprecates dc1's only valid records.
No precondition check that step 1 ran for the DC being operated on.
--dc accepts choices=sorted(C.DC_GUA), i.e. BOTH vr1-dc0 and vr1-dc1
(d139-step6-vip-rehome.py:152). vr1-dc1 is one keystroke from vr1-dc0.
MEASURED, the apex's vr1-dc1 GUA carve is INCOMPLETE -- only 4 rows exist under
2602:f3e2:f03::/48, all provider-public:
2602:f3e2:f03::/48 vr1-dc1 dc (container)
2602:f3e2:f03:10::/60 vr1-dc1 provider-public
2602:f3e2:f03:10::/64 vr1-dc1 provider-public
2602:f3e2:f03:11::/64 vr1-dc1 provider-public
There is NO 2602:f3e2:f03:20::/64 and NO 2602:f3e2:f03:21::/64.
MEASURED, the tool nevertheless plans (dry run, rc=0, no warning of any kind):
D-139 STEP 6 -- vr1-dc1 -- apex http://10.10.1.10:8000
CREATE 26 | ALREADY 0 | DEPRECATE-ADDR 26 | DEPRECATE-PFX 9
... 2602:f3e2:f03:20::50/64 .. :20::62, 2602:f3e2:f03:21::50 .. :21::62
DEPRECATE prefixes: fd50:840e:74e2:320::/60, :320::/64, :321::/64,
:330::/60, :330::/64, :340::/60, :340::/64,
:350::/60, :350::/64
With --commit this would write 26 ip-addresses into /64s that DO NOT EXIST as prefix
rows, then mark dc1's 26 authoritative ULA addresses and 9 ULA prefixes deprecated,
and exit 0. dc1's VIPs would then be recorded ONLY as orphans with no containing
prefix, while every row that currently describes them reads unusable.
This is exactly the orphaning hazard DEFECT 3 of the 2026-08-01 CORRECTION NOTE
names, and exactly what the ordering ruling exists to prevent -- reached not by a
race or a partial failure but by the tool's own documented interface.
Note the asymmetry that makes this easy to miss: gua_net_for_role() computes the
target /64 ARITHMETICALLY from C.DC_GUA and C.CARVE. It never asks the apex whether
that /64 exists. For vr1-dc0 all 16 target prefixes happen to exist (step 1 applied),
so dc0's dry run looks perfect and hides the missing check entirely.
Compare the imported tool: d139-gua-carve.py checks its preconditions before writing
(exactly one site with the slug, line 177-180; every CARVE role exists in the apex,
line 183-186, "refusing to half-carve a DC"). d139-step6-vip-rehome.py checks none.
FIX (suggested, operator's call): before planning, require that every GUA /64 the plan
targets EXISTS as a prefix row scoped to --dc, and die() if not, naming step 1.
Optionally also restrict --dc to vr1-dc0 for this window.
DEF-2 [CRITICAL -- defect MEASURED, consequence REASONED]
The SANDBOX_HOSTS apex-identity guard present in the imported tool is ABSENT from
the new tool. It will write to whatever NETBOX_URL names.
d139-gua-carve.py carries the guard:
:25 SANDBOX_HOSTS = {"localhost", "127.0.0.1", "10.10.1.10"}
:161 if host not in SANDBOX_HOSTS:
:162 die("'%s' is not the VR1 working apex. office1-netbox (10.10.1.10) takes ALL
VR1 reads and writes; netbox.baldurkeep.com is the v1 REFERENCE and stays
untouched (DOCFIX-195)." % host)
d139-step6-vip-rehome.py: grep -c SANDBOX_HOSTS -> 0. No urlsplit, no hostname check.
The guard lives inside d139-gua-carve.py's main(), which the import never executes, so
subclassing C.NB inherits the transport but NOT the safety posture.
MEASURED differential against a non-sandbox host (127.0.0.9:1, unreachable, dummy
token -- nothing real was contacted):
carve: REFUSE: '127.0.0.9' is not the VR1 working apex. ... (DOCFIX-195) rc=2
step6: REFUSE: apex unreachable or rejecting: GET /ipam/prefixes/?limit=500
-> <urlopen error [Errno 111] Connection refused> rc=2
Both exit 2 here only because the host is unreachable. The carve tool refused on
IDENTITY, before any network call; the step-6 tool refused on REACHABILITY. Against a
REACHABLE non-sandbox apex -- netbox.baldurkeep.com, the v1 REFERENCE that DOCFIX-195
says stays untouched -- the step-6 tool would proceed to plan and, with --commit,
write. One wrong `.` of an env file is the whole distance.
This defect is a REGRESSION against the imported module's own stated posture, which is
the strongest argument for fixing it: the reviewer of step 1 already decided this
guard was required for a WRITE tool against this apex.
FIX: lift the same guard (reuse C.SANDBOX_HOSTS, do not retype the set).
DEF-3 [HIGH, MEASURED] SILENT UNDER-COUNT: a missing or mis-scoped ULA /64 prefix row
silently halves the work and the run still exits 0.
dep_addrs and creates are derived ONLY from addresses contained in a /64 prefix ROW
that is scoped to --dc and inside the retired ULA /48 (plan(), lines 102-141). The
addresses themselves are never counted independently, and no expected total is
asserted anywhere.
MEASURED offline against the pristine repo file, full 26-address fixture, with only
the fd50:840e:74e2:221::/64 PREFIX row removed (its 13 addresses still present):
D1 the :221::/64 prefix ROW is absent
-> CREATE=13 DEPA=13 DEPP=8 ALREADY=0 -- no complaint, plan() returns
D1b the :221::/64 row's scope is blank/name-only instead of the slug
-> CREATE=13 DEPA=13 DEPP=8 ALREADY=0 -- same
Thirteen live VIPs would be neither created nor deprecated, and the run would report
success. D1b is not hypothetical shape-wise: scope_slug() itself exists because the
live API returns scope.slug while repo dumps normalise scope.name, and its own
docstring records that matching on name alone "found ZERO live".
This is the same class as CORRECTION NOTE DEFECT 2 -- dc-node-v6-carve.py carving FOUR
FEWER PLANES PER NODE and EXITING CLEAN -- and the same class as the MEMORY entry
"instrument currency before negatives". The carve tool guards its half of this
(build_retire(): "the apex returned ZERO prefixes scoped to %s ... refusing to call
every plane 'missing' on that evidence"). The step-6 tool guards only the total-zero
case (`if not ula_pfx`), not the partial case, which is the one that actually bites.
FIX: assert a floor. The ruling records 26 / 26 / 9 as MEASURED; make the tool require
it (e.g. --expect-creates / --expect-deprecate-addrs / --expect-deprecate-prefixes,
or cross-check dep_addrs against the count of ALL v6 addresses inside the retired /48
that are NOT in another DC's scoped prefixes). Any tool that can do less than the
ruling specifies and still exit 0 is not gated.
DEF-4 [HIGH, MEASURED] The HARNESS does not test main() at all -- so the CREATE ->
verify -> DEPRECATE ORDER, the one property both rulings name, is UNCOVERED.
All of T1-T12 call plan(). Nothing calls or inspects main(). The harness header states
"the tool's write path is CREATE -> verify -> DEPRECATE ... plan() is what decides all
three sets, so it is where the risk lives" -- that sentence is the bug. plan() decides
the SETS; main() decides the ORDER, and ORDER is what the ruling constrains:
"CREATE FIRST, VERIFY, THEN DEPRECATE -- reversed, there would be an interval in which
the apex marks a live VIP's only record unusable."
MUTATION M1b, MEASURED. Into a COPY of the tool, immediately before the "PHASE 1:
CREATE" block, inserted:
for _x in dep_addrs:
nb.patch("/ipam/ip-addresses/%d/" % _x["id"], {"status": DEPRECATED})
for _x in dep_prefixes:
nb.patch("/ipam/prefixes/%d/" % _x["id"], {"status": DEPRECATED})
i.e. the tool now deprecates every ULA row BEFORE creating a single GUA record --
precisely the forbidden ordering. Harness result:
RESULT: PASS=14 FAIL=0
ALL PASS
The suite cannot see the defect it was written to prevent.
FIX: test main() with a fake NB that RECORDS the call sequence, and assert that no
PATCH is issued before every POST has been issued and read back. Also assert the
failure branches: a POST that raises leaves zero PATCHes; a read-back that comes back
short leaves zero PATCHes.
ORDERING IN THE SOURCE IS, HOWEVER, CORRECT AS WRITTEN (read, not executed): phase 1
posts all creates; a RuntimeError returns 1 with "NOTHING WAS DEPRECATED"; phase 2
re-reads /ipam/ip-addresses/ and returns 1 on any missing create, again before any
patch; phase 3 patches; phase 4 re-reads and verifies status. I found no path in the
source where a ULA row is deprecated while its GUA counterpart does not exist, EXCEPT
the vacuous case described in DEF-6 below. This is REASONED from source plus the
MEASURED dry run; the --commit path was not executed, per the review constraint.
DEF-5 [MEDIUM, MEASURED] T13, the case that names itself PROOF-OF-TEETH, is an assertion
on ABSENCE and therefore passes vacuously on ANY error in its own snippet.
T13 is `if grep -q "MUTANT_FIRST=<correct>" ; then bad ; else ok ; fi`. Any output that
lacks that string satisfies it -- including a traceback.
MUTATION M0, MEASURED. The tool file was replaced (in the COPY) with one line of
non-Python. Result:
FAIL T1 .. FAIL T12 (all report "Traceback (most recent call last):")
PASS T13 PROOF-OF-TEETH: a wrong /48 changes the target, so T2 is a real assertion
PASS T14 tool contains NO delete path
RESULT: PASS=2 FAIL=12
A tool that does not even parse passes the teeth test. Standing rule, script-authoring
reference: "prove each new assertion can FAIL *and* prove it RAN."
FIX: make T13 assert POSITIVELY -- require a MUTANT_FIRST line to be present AND to
differ from the correct target (two greps, not one negation).
DEF-6 [MEDIUM, MEASURED offline; NOT live today; reachable AFTER a first --commit run]
A pre-existing but DEPRECATED GUA record is accepted as ALREADY, and its ULA
counterpart is then deprecated too -- leaving the VIP with no usable apex row.
plan() builds `existing` as a set of bare IPs (line 113) and the phase-2 read-back
likewise compares bare IPs only (lines 205-210). Neither consults status.
MEASURED offline, pristine tool, full fixture plus one deprecated GUA record:
D3 GUA counterpart exists but is DEPRECATED
-> CREATE=25 DEPA=26 DEPP=9 ALREADY=1
::50 in creates? False ULA ::50 in dep_addrs? True
After commit BOTH the ULA and the GUA record for that VIP read deprecated.
Not live now (there are zero deprecated addresses in the apex). It becomes reachable
the moment step 6 has run once and anyone flips a GUA row to deprecated -- i.e. it is
an IDEMPOTENCE hazard on re-run, not purely theoretical. It is also the only route I
found by which a ULA row ends up deprecated without a usable GUA counterpart, so it
bears directly on the brief's item 1.
FIX: treat a non-reserved/non-active existing GUA record as a REFUSE, or PATCH it back
to VIP_STATUS as part of the create phase, and include status in the read-back
predicate.
DEF-7 [MEDIUM, MEASURED] T14's delete-path assertion covers ONE file, not the surface
the tool actually has.
Item 6 of the brief, answered directly: AS WRITTEN TODAY there is NO delete path.
Verified by reading both files -- the only HTTP methods reachable are GET (get_all),
POST (post) and PATCH (NBW.patch). C.NB._req takes an arbitrary `method` string but no
caller passes DELETE. That is a clean finding.
The ASSERTION that keeps it that way, however, greps "$TOOL" only.
MUTATION M2, MEASURED. A delete() method was added to the COPY of the IMPORTED module
d139-gua-carve.py (whose NB class step 6 subclasses), leaving the step-6 file untouched:
def delete(self, path):
return self._req("DELETE", path)
RESULT: PASS=14 FAIL=0
T14 stays green while the class the tool inherits from has grown a delete path. The
ruling "Deprecate both, delete nothing" is repo-wide; the assertion is file-wide.
FIX: grep both netbox/d139-step6-vip-rehome.py and netbox/d139-gua-carve.py, or assert
on the method set reachable from NBW.
DEF-8 [LOW, MEASURED offline; NOT live] An address inside a ULA /60 parent but outside
its /64 is silently dropped while the /60 is deprecated anyway.
plan() skips any prefix with prefixlen != 64 (line 117-118) -- correct, and necessary:
MUTATION M6 (removing that guard) makes CREATE jump 26 -> 52 and T1/T6/T7 all fire.
THE GUARD IS DOING MORE THAN PREVENTING A DOUBLE-COUNT, and this is worth stating
because it changes how much weight it carries: fd50:840e:74e2:221::/64 (metal-internal)
is CONTAINED IN fd50:840e:74e2:220::/60 (role metal-admin). With the guard removed, the
/60 matches all 26 addresses and maps the 13 metal-INTERNAL VIPs onto
2602:f3e2:f02:20::/64, the metal-ADMIN GUA plane -- 26 + 13 + 13 = 52, exactly the
measured figure. So that one `continue` is the only thing standing between the tool and
a silent CROSS-PLANE MIS-MAP. It must never be relaxed.
But dep_prefixes deprecates ALL in-scope ULA prefixes including the /60s. MEASURED,
pristine tool, fixture plus one address at fd50:840e:74e2:225::7 (inside :220::/60,
outside :220::/64):
CREATE=26 DEPA=26 DEPP=9
stray created? False stray deprecated? False
BUT its /60 IS deprecated: True
NOT LIVE: all 52 ULA addresses in the apex sit inside a /64. Latent only.
DEF-9 [LOW, MEASURED offline; NOT live] Duplicate prefix ROWS for the same CIDR produce
duplicate CREATE payloads.
NetBox permits two prefix rows with the same CIDR. plan() iterates rows, not CIDRs, and
the `already` check is against the pre-read `existing` set, never against creates
itself. MEASURED, pristine tool, fixture plus a second row for :220::/64:
CREATE=39 DEPA=39 DEPP=10
duplicate CREATE payloads: 13 (e.g. 2602:f3e2:f02:20::50/64)
NOT LIVE: zero duplicate CIDRs measured apex-wide. Theoretical.
DEF-10 [LOW, REASONED] ip_network.subnet_of() is true for a network against ITSELF, so if
fd50:840e:74e2::/48 ever exists as a prefix ROW scoped to a DC it would be deprecated
by this tool -- the org aggregate d139-gua-carve.py:48 calls "still a valid org
aggregate". MEASURED: no such row exists today (the 18 in-/48 rows are all /60 or /64).
Latent only; stated because the /48 is explicitly still-valid per the carve tool.
DEF-11 [COSMETIC, MEASURED] tests/.../run-tests.sh defines run_case() at line 23-25 and
never calls it. Dead code in a harness whose job is to be trusted.
-------------------------------------------------------------------------------
ASSERTIONS THAT SURVIVED MUTATION
-------------------------------------------------------------------------------
Mutation battery, each applied to a COPY, harness re-run, restored between runs:
M0 tool replaced with unparseable text ..... T13, T14 PASS (see DEF-5)
M1b deprecate loops moved before creates .... ALL 14 PASS (see DEF-4)
M2 delete() added to imported carve module.. ALL 14 PASS (see DEF-7)
M3 VIP_STATUS reserved -> active ........... T4 fires
M4 ALREADY dedup removed ................... T6 fires
M5 ambiguity REFUSE -> pick first hextet ... T10 fires
M6 /60-parent skip removed ................. T1, T6, T7 fire
M7 dep_prefixes forced empty ............... T1, T7, T12 fire
M8 already-deprecated filter removed ....... T7 fires
CONCLUSION: T1, T4, T6, T7, T10, T12 have real teeth against plan(). T2/T3 are proven
by T13's construction (M6/M7 also move them indirectly). T13 and T14 are the two that
survive everything (DEF-5, DEF-7), and NO assertion at all covers main() (DEF-4).
CASE COUNT: claimed T1-T14; measured PASS=14 FAIL=0; count and labels agree. The
suite's cases all RAN. (Standing rule "assert the case count moved" -- satisfied.)
Note on coupling: T1-T5 all grep one shared $out from a single python run. A crash in
that run turns five cases red together, which is correct but means the five are not
five independent signals.
-------------------------------------------------------------------------------
WHAT I COULD NOT CHECK
-------------------------------------------------------------------------------
1. THE --commit WRITE PATH WAS NEVER EXECUTED (review constraint). Phases 1-4 are
REASONED from source only. No POST, PATCH or read-back behaviour is measured. In
particular I did not observe how the apex responds to a duplicate ip-address POST,
nor whether enforce_global_unique is set on this NetBox.
2. MAAS. The D-139 execution list step 6 reads "retire the ULA rows IN THE APEX AND IN
MAAS". This tool is apex-only and its docstring does not claim otherwise. I did not
check whether the MAAS half is covered by another tool or is still owed. FLAGGED as
an open scope question for the operator, not as a defect of this tool.
3. Whether --dc vr1-dc1 is intended to be run in this window at all. DEF-1 is stated as a
missing guard regardless of intent; if dc1 is out of scope the fix may simply be to
restrict the flag.
4. C.NB._req with method="DELETE" was never exercised; I verified by reading that no
caller passes it, not by running it.
5. Real-world contents of ~/vr1-office1-creds/vr1-netbox-sandbox.env -- sourced only,
never read or printed, per the secrets rule.
6. Concurrency. If anything else writes the apex between the tool's initial read and its
phase-3 patch, plan() is stale. Not evaluated.
7. The 2602:f3e2:f02:11:: provider-public VIP records' provenance -- I confirmed they
EXIST and match the overlay, but did not trace which tool created them.
-------------------------------------------------------------------------------
REPO INTEGRITY STATEMENT
-------------------------------------------------------------------------------
No repo file was edited by this review. All mutation experiments were performed on
copies under /home/jessea123/.claude/jobs/015c103e/tmp/mut/ , and the pre-mutation
copies were diffed against the repo originals at the end of the run:
IDENTICAL: netbox/d139-step6-vip-rehome.py
IDENTICAL: netbox/d139-gua-carve.py
IDENTICAL: tests/d139-step6-vip-rehome/run-tests.sh
The harness was re-run against the real repo files afterwards: PASS=14 FAIL=0.
bash scripts/repo-lint.sh -> 0 fail, 1 warn (the pre-existing L1 legacy-ASCII warn on
docs/design-decisions.md), 657 files scanned.
NOTE, stated because it would otherwise look like my doing: `git status --short`
showed ONLY the two untracked targets at the start of this review, and later showed
docs/CURRENT-STATE.md, docs/design-decisions.md and tests/HARNESS-MANIFEST as modified.
I inspected those diffs: they are the status=active -> status=reserved correction and
the HARNESS-MANIFEST entry for d139-step6-vip-rehome -- concurrent edits from the
parent session, not from this review. No git add / commit / push was run.
No cloud mutation of any kind was performed. All apex access was GET.
-------------------------------------------------------------------------------
SUMMARY OF REQUIRED FIXES BEFORE --commit
-------------------------------------------------------------------------------
MUST DEF-1 precondition: every target GUA /64 must exist as a prefix row for --dc
MUST DEF-2 restore the SANDBOX_HOSTS apex-identity guard (reuse C.SANDBOX_HOSTS)
MUST DEF-3 assert the expected 26 / 26 / 9 floor; partial must not exit 0
MUST DEF-4 harness must cover main()'s ordering and its two failure branches
SHOULD DEF-5 make T13 assert positively so it cannot pass on a traceback
SHOULD DEF-6 include status in the ALREADY / read-back predicate
SHOULD DEF-7 extend T14's grep to the imported module
MAY DEF-8..DEF-11 latent / cosmetic; record rather than fix if time is short
The mapping logic, the never-delete posture, the status choice, the verbatim
descriptions and the source-level phase ordering are all correct and were verified.
This is a good tool missing its preconditions and missing the test for its own
headline property.
