Repository navigation
rigs: trust store via the harness storage API; retire _temp_gc.py - #293
Merged
Merged
Conversation
rig_trust_store.py and rig_trust_policy.py now read, write and delete TRUST.A / TRUST.B through c64-test-harness's storage_* API (harness #526) instead of a private ftplib session. tools/uci/_trust_storage.py holds the shared code: - the store directory is the image's, /<volume>/c64https-test only; an explicit TRUST_STORE_DIR must match it; - the volume must be in storage_volumes() minus WRITE_REFUSED_VOLUMES, compared casefolded (the device says "Flash", the set holds "flash"); - no writable volume, or Ultimate64StorageVolumeError/StorageError during setup, is COULD NOT RUN, exit 2, naming the available volumes, before an Ultimate64Client is built; - a zero-byte trust file is refused by name (the format's minimum is 16 B; the firmware would create no file). The API has no listing or rmdir: the rigs only touch TRUST.A/TRUST.B and leave the empty directory. rig_trust_store's phase 14 (FAIL NOPATH) now points the resident ts_name at a missing sibling directory instead of removing the real one; it is not run on the cold bank. tools/uci/_temp_gc.py is removed with its call sites (live, wiki, banner, refetch, close_retry, m3_demo, both trust rigs): the harness sweeps /Temp for any default Ultimate64Client on a leak-prone device while the DeviceLock is held, and every one of those rigs builds its client that way inside the lock. tools/test_trust_rig_storage.py (pytest testpaths, faked storage, no hardware) pins the volume selection, the clean exit 2 of both rigs, the zero-byte refusal and the absence of ftplib/_temp_gc. Against the master rigs it fails 6 of 16 cases; all pass after. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…CLOSED Review round on 5ffd7be, with the harness's f384ad0 (PR #527): - select_store_dir uses storage_writable_volumes instead of our own casefold filter. - check_only_ours restores the "nothing but TRUST.A/B" guard with storage_list_dir, and probes TRUST.P by name (listings omit hidden names; a TRUST_BUNDLE build reads TRUST.P). A foreign directory is exit 2 and is left untouched. - rig_trust_store phase 14 (FAIL NOPATH) is a real storage_rmdir of the test directory again, then put back, so the cold-bank comb product is covered; the ts_name RAM poke is gone. Cleanup removes the directory; Ultimate64StorageNotEmptyError is reported (hidden entries when .entries is empty), never forced. - Exit codes narrowed: only StoreUnavailable / a volume error is COULD NOT RUN (2). Lock refusals, FTP failures and other storage errors are "[fatal] storage", exit 1. - rig_trust_policy phases 2 and 6 waited 240 s for "DONE", which the program never prints; they wait for done_msg "CONNECTION CLOSED" (src/boot.s) and the result is now a check. - rig_https_m3_demo no longer FTP-deletes its /Temp/cache/upload image: that is the firmware's own managed upload directory. - docs/engineering-notes.md: the rig_https_live entry and the wedge note no longer describe _temp_gc.py as live. tools/test_trust_rig_storage.py: the fake storage refuses and records a violation unless the fake DeviceLock is held. 20 cases; 9 fail against 5ffd7be, all pass now. Mutants killed: lock.release() first in either rig's finally, TrustFiles.get catching Exception, every storage error treated as COULD NOT RUN, the TRUST.P probe dropped, cleanup without rmdir, the zero-byte guard dropped. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…D tests Second review round, on 729b111: - Correction to 729b111's message: the new suite failed 8 of 20 cases against 5ffd7be, not 9 (the count included the FAILED summary line). - One policy for hidden leftovers in both rigs: a test directory that cannot be removed (StoreUnavailable subclass StoreLeftovers, raised by TrustFiles.remove_dir on Ultimate64StorageNotEmptyError) is device state, COULD NOT RUN, exit 2. Setup (TrustFiles.prepare) now proves the directory removable before the device is touched; phase 14 meeting it later is exit 2 too, not a failed check. - Cleanup failures are no longer swallowed: clean_up() + final_exit() turn a failed delete/rmdir into exit 1 and leftovers into exit 2; a failed check (1) still wins. - rig_trust_store resets the C64 before cleanup, so the 6502 cannot be mid-DOS-write while the files and directory are removed. - rig_trust_policy: the CLOSED wait + check is await_closed(); the comment now says done_msg also follows a send failure and a truncated body (it proves the fetch ended, not that it succeeded). Both rigs note that LockTimeoutConfigError -> 2 predates this work. tools/test_trust_rig_storage.py, now 32 cases (10 fail against 729b111): phase 14 offline (rmdir before the load, ensure_dir after it, stop on a false rmdir result, the NOPATH reason), await_closed and its two uses in phases_local, full offline runs of both rigs (pass + rmdir twice, cleanup failure -> 1, leftovers -> 2, phase-14 leftovers -> 2, reset before cleanup). 16 mutants, all killed, including: ensure_dir dropped, rmdir result ignored, CLOSED check removed, cleanup failure swallowed, cleanup moved before the reset. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Setup (TrustFiles.prepare) proves the store directory empty and removable, and the DeviceLock excludes other FTP writers, so an entry still there at phase 14 or at end-of-run cleanup was written DURING the run -- by the program under test. That is product behaviour: - clean_up() reports leftovers as (1, msg) naming the entries; phase 14 turns StoreLeftovers into a failed check naming them. Both rigs exit 1 (cc012c3 exited 2; a stray visible TRUST.C at cleanup showed it). - Exit 2 stays only for leftovers found at setup, whose message now says they predate the run. - final_exit() loses its exit-2 branch; rig_trust_store's "blocked" path and rig_trust_policy's dead `if run is None` branch are gone. tools/test_trust_rig_storage.py, 33 cases, 4 failing against cc012c3: cleanup leftovers (visible TRUST.C and hidden) -> exit 1 in both rigs, phase-14 leftovers -> a failed check naming TRUST.C, setup leftovers say "before this run". 17 mutants, all killed. U64E rerun of rig_trust_policy.py on cc012c3 (PRG d8254882...): 30 passed, 0 failed, exit 0. This commit changes only the leftover exit path, which that run did not reach. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The trust rigs (
rig_trust_store.py,rig_trust_policy.py) move off their privateftplibsession onto c64-test-harness's storage API (harness #526/#527, master f384ad0); shared code intools/uci/_trust_storage.py.tools/uci/_temp_gc.pyis retired with its call sites in 8 rigs (the harness now sweeps /Temp under the DeviceLock), andrig_https_m3_demono longer deletes the firmware-managed/Temp/cache/upload. Also fixes a pre-existing bug:rig_trust_policywaited 240 s for "DONE", which the program never prints (it prints "CONNECTION CLOSED").Exit policy: leftovers at setup -> 2 (COULD NOT RUN, predates the run); leftovers mid-run or at cleanup -> 1 (the program under test wrote them); cleanup failures -> 1, never swallowed; the C64 is reset before cleanup.
Evidence
tools/test_trust_rig_storage.py(pytest testpaths, faked storage that refuses calls without the fake DeviceLock): 33 cases. Red against each prior round: 6/16 vs master, 8/20 vs 5ffd7be (729b111's message said 9; corrected in cc012c3), 10/32 vs 729b111, 4/33 vs cc012c3. 17 mutants, all killed.d8254882…(comb cold-bank,TRUST_STORE_DIR=/USB1/c64https-test HTTPS_PORT=4433): U64Erig_trust_store.py25/0 exit 0; U64Erig_trust_policy.py30/0 exit 0 (rerun on cc012c3; e0f348d changes only the leftover path, unreached on a clean run). C64U: both rigs exit 2 by design: no writable volume (empty /SD; Flash and Temp only).Merge order: before the P-384 retirement PR, which conflicts in
tools/uci/README.mdand will be rebased onto this.Commits (messages verbatim; each carries its red-green evidence)
5ffd7be rigs: trust store via the harness storage API; retire _temp_gc.py
rig_trust_store.py and rig_trust_policy.py now read, write and delete
TRUST.A / TRUST.B through c64-test-harness's storage_* API (harness
#526) instead of a private ftplib session. tools/uci/_trust_storage.py
holds the shared code:
explicit TRUST_STORE_DIR must match it;
compared casefolded (the device says "Flash", the set holds "flash");
setup, is COULD NOT RUN, exit 2, naming the available volumes, before
an Ultimate64Client is built;
16 B; the firmware would create no file).
The API has no listing or rmdir: the rigs only touch TRUST.A/TRUST.B
and leave the empty directory. rig_trust_store's phase 14 (FAIL NOPATH)
now points the resident ts_name at a missing sibling directory instead
of removing the real one; it is not run on the cold bank.
tools/uci/_temp_gc.py is removed with its call sites (live, wiki,
banner, refetch, close_retry, m3_demo, both trust rigs): the harness
sweeps /Temp for any default Ultimate64Client on a leak-prone device
while the DeviceLock is held, and every one of those rigs builds its
client that way inside the lock.
tools/test_trust_rig_storage.py (pytest testpaths, faked storage, no
hardware) pins the volume selection, the clean exit 2 of both rigs,
the zero-byte refusal and the absence of ftplib/_temp_gc. Against the
master rigs it fails 6 of 16 cases; all pass after.
729b111 rigs: trust storage onto harness #527; list/rmdir guards; CONNECTION CLOSED
Review round on 5ffd7be, with the harness's f384ad0 (PR #527):
casefold filter.
storage_list_dir, and probes TRUST.P by name (listings omit hidden
names; a TRUST_BUNDLE build reads TRUST.P). A foreign directory is
exit 2 and is left untouched.
test directory again, then put back, so the cold-bank comb product is
covered; the ts_name RAM poke is gone. Cleanup removes the directory;
Ultimate64StorageNotEmptyError is reported (hidden entries when
.entries is empty), never forced.
NOT RUN (2). Lock refusals, FTP failures and other storage errors are
"[fatal] storage", exit 1.
program never prints; they wait for done_msg "CONNECTION CLOSED"
(src/boot.s) and the result is now a check.
that is the firmware's own managed upload directory.
no longer describe _temp_gc.py as live.
tools/test_trust_rig_storage.py: the fake storage refuses and records a
violation unless the fake DeviceLock is held. 20 cases; 9 fail against
5ffd7be, all pass now. Mutants killed: lock.release() first in either
rig's finally, TrustFiles.get catching Exception, every storage error
treated as COULD NOT RUN, the TRUST.P probe dropped, cleanup without
rmdir, the zero-byte guard dropped.
cc012c3 rigs: trust storage exit policy, reset before cleanup, phase-14/CLOSED tests
Second review round, on 729b111:
against 5ffd7be, not 9 (the count included the FAILED summary line).
cannot be removed (StoreUnavailable subclass StoreLeftovers, raised by
TrustFiles.remove_dir on Ultimate64StorageNotEmptyError) is device
state, COULD NOT RUN, exit 2. Setup (TrustFiles.prepare) now proves the
directory removable before the device is touched; phase 14 meeting it
later is exit 2 too, not a failed check.
turn a failed delete/rmdir into exit 1 and leftovers into exit 2; a
failed check (1) still wins.
mid-DOS-write while the files and directory are removed.
comment now says done_msg also follows a send failure and a truncated
body (it proves the fetch ended, not that it succeeded). Both rigs note
that LockTimeoutConfigError -> 2 predates this work.
tools/test_trust_rig_storage.py, now 32 cases (10 fail against 729b111):
phase 14 offline (rmdir before the load, ensure_dir after it, stop on a
false rmdir result, the NOPATH reason), await_closed and its two uses in
phases_local, full offline runs of both rigs (pass + rmdir twice,
cleanup failure -> 1, leftovers -> 2, phase-14 leftovers -> 2, reset
before cleanup). 16 mutants, all killed, including: ensure_dir dropped,
rmdir result ignored, CLOSED check removed, cleanup failure swallowed,
cleanup moved before the reset.
e0f348d rigs: leftovers after setup are a failure, not COULD NOT RUN
Setup (TrustFiles.prepare) proves the store directory empty and
removable, and the DeviceLock excludes other FTP writers, so an entry
still there at phase 14 or at end-of-run cleanup was written DURING the
run -- by the program under test. That is product behaviour:
turns StoreLeftovers into a failed check naming them. Both rigs exit 1
(cc012c3 exited 2; a stray visible TRUST.C at cleanup showed it).
they predate the run.
path and rig_trust_policy's dead
if run is Nonebranch are gone.tools/test_trust_rig_storage.py, 33 cases, 4 failing against cc012c3:
cleanup leftovers (visible TRUST.C and hidden) -> exit 1 in both rigs,
phase-14 leftovers -> a failed check naming TRUST.C, setup leftovers
say "before this run". 17 mutants, all killed.
U64E rerun of rig_trust_policy.py on cc012c3 (PRG d8254882...): 30
passed, 0 failed, exit 0. This commit changes only the leftover exit
path, which that run did not reach.
🤖 Generated with Claude Code