fix: close ten v0.6.0 triage issues — flash MRAM guard, Zephyr executor parity, the emit-site gate, and two host-dependent parity suites - #331
Closed
alpCaner wants to merge 12 commits into
Closed
Conversation
The two ensure_venv_* tests write their stand-in interpreter with std::fs::write and let ensure_venv spawn it immediately. cargo test runs tests in parallel threads, so a sibling test's fork can still hold a write fd on the just-written file and the spawn fails ETXTBSY. ensure_venv fails CLOSED on a probe error and recreates the venv, so the assertion that the venv was REUSED then panics -- a required check (test (ubuntu-latest)) going red with no code change, at a measured 1 in 20. wait_until_spawnable already exists in this file for exactly this reason: 204c3a0 added it to exit_code_shim to cure the same race for a sibling test, but did not touch these two, which still write their shim inline. crates/ is frozen for feature work; this is the deliberate test-hermeticity exception 204c3a0 set the precedent for. Zero production delta. Measured post-fix: 20/20 green over a full-binary loop.
…sal causes (#311, #312) plan_alif_mram_jlink emitted `loadbin <artefact> <app_address>` uncondi- tionally, with no is_raw_bin guard -- unlike its sibling plan_swd_probe, which branches loadbin-for-.bin vs loadfile-for-ELF. A slot0-linked artefact that is not a raw .bin would have had its ELF headers written into on-die MRAM at the load address. It now raises FlashPlanError naming the artefact and pointing at zephyr.bin. Deliberately a refusal rather than plan_swd_probe's loadfile fallback: a loadfile here would silently ignore slot0_load_address, which is the worse failure. Latent today -- no in-tree board.yaml sets slot0_load_address. Separately, the Flow D DPIDR preflight conflated two causes. A banner reporting a DIFFERENT DP ID (a real wrong-board or probe-selection problem) and a banner reporting no DP ID at all (connect failed outright, typically the J-Link still re-enumerating after a prior JLinkExe close) both got: Check the probe selection (flash_args.jlink_serial) and the wiring. The second now says the probe reported no DP ID at all and advises a retry. Detection is conservative and falls back to the existing generic message when it cannot tell -- a widened refusal that claimed the wiring was fine would be worse than the ambiguity. Both cases still refuse to write MRAM; this is a diagnostic split only. Neither backend has a v0.4.1 oracle counterpart -- alif_mram_jlink is new -- so there is no envelope to diff against here.
…at never loaded Zephyr (#308, #309) Two oracle-parity divergences in the executor, where the v0.4.1 oracle does the right thing and the port did not. #308: build_cmd.py passed gap_fillers=() with a literal '# NOT YET PORTED', and execute.py starts from dict(os.environ). ADR-0020 plans never carry ZEPHYR_BASE, so a stale ambient $ZEPHYR_BASE in the user's shell beat the west workspace tan had itself just resolved. The oracle fills it via zephyr_env_overrides; the resolved value already existed on this side as west_workspace_dir. Ported as pure logic into python/tan/core/zephyr_env.py rather than into the command module. EXTRA_ZEPHYR_MODULES joins with ';' on every platform -- it is a CMake list that Zephyr's own zephyr_module.py splits on ';' regardless of host OS, and joining it with ':' on Linux breaks west build configure with 'is not a valid zephyr module'. The values flow through the existing sep_for_key machinery rather than re-implementing the join, and assemble_slice_env's seed-then-append still extends an inherited PYTHONPATH instead of replacing it. Both are covered by tests. #309: a CMakeLists.txt that never calls find_package(Zephyr ...) still configures and links fine under `west build -b <board>` -- CMake only emits a dev warning about the missing project() call -- so a real exit code 0 was not sufficient evidence and a core declared `os: zephyr` could produce a plain host binary and be reported [+] ok. The board name is never even validated, because nothing loaded the code that would validate it. zephyr_boilerplate_loaded / dir_shows_zephyr are ported into build/ manifest.py and applied in execute.py's status assembly, checked only on an otherwise-successful slice and skipped when the slice redirects west's build dir, where the evidence lives somewhere this cannot see. It looks one level below build/ as well, because --sysbuild nests the real per-image Zephyr builds under its own superbuild and only the nested build carries a signal. The guard exposed 13 tests whose plan fixtures declared backend zephyr while dispatching a trivial stand-in tool that produces no Zephyr evidence. Twelve are relabelled to a non-zephyr backend after checking per test that no assertion touched zephyr artefact resolution; the one test genuinely about west's own build-dir resolution instead has its stand-in west script create a real zephyr/ directory, keeping it on the live code path. The separate question of whether `tan init --template minimal-app` should emit a different CMake shape is a maintainer decision and is untouched here; the scaffold is byte-identical to the oracle's.
The Rust gate for this landed in 78d8308 (contract.rs's every_prefixed_issue_code_is_registered, with PREFIXING_SITES), but crates/ ships to nobody -- the release assets are PyInstaller freezes of python/ -- so on the shipping surface there was no source-to-registry scan at all, prefixed or otherwise. The shape the Rust gate exists to catch is already live here: bootstrap_cmd builds f"bootstrap.{code}", debug_config_cmd builds f"debug-config.{code}", doctor_cmd does the same. A literal-only scan sees none of them. The new gate walks emit sites for both literal codes and f-string assembly through a prefixing helper, reading the registry through the same source test_frozen_issue_codes.py already uses rather than inventing a second one. It carries a self-test proving it REJECTS a deliberately unregistered code: a gate that cannot fail is not a gate, which this repo learned for real in tan-cli#275 when a scrubbed env var made one skip silently for eleven commits. Where it must skip, it skips loudly and names what was missing. Run against the tree it found 146 unregistered codes, including cli.command-deferred. All are registered `reserved` in contract/ issue-codes.json -- 146 additions, zero removals, zero modifications. cargo test --locked stays green, frozen_issue_codes included. contract/issue-codes.json ships as a release asset; the next release notes should name these entries.
…nventory (#313, #324) Eight parity cases branch on a which() probe, so their frozen answers record which tools the CAPTURE host had -- and the Python side is then compared live against the REPLAY host. The two sides were not measuring the same environment, so the comparison measured the host rather than the port. #313: three yocto_wic cases froze 'would run dd if=...', because the capture host had dd and no bmaptool. plan_yocto_wic prefers bmaptool when planning only and dd is absent, so on a host with neither the live Python side picked bmaptool and the three failed. Green wherever dd exists, hence green in CI. #324: three west_forward cases froze the 'west not found on PATH' launch error, because the capture host had no west. On a host that HAS west the live Python side genuinely launches it and reports the child's failure instead. Same bug, opposite direction. Neither is a port gap. Run both binaries under a sanitised environment and they agree byte for byte -- same command, same exitCode, same data, same issue code, same message. The port already carries the identical check and the identical string. Replay now pins the Python side's PATH to the recorded inventory. Because that side runs as a subprocess, an in-process monkeypatch cannot reach it, so the pin crosses the process boundary as a scratch PATH. #324 reuses the mechanism #313 introduced rather than adding a second one, generalised to the absent-tool case as oracle.empty_tool_inventory. Under TAN_PARITY_LIVE=1 both sides genuinely spawn, and compare() REFUSES a python_env_overrides rather than silently comparing one pinned side against one unpinned side -- which would be a fresh divergence, not a capture. PROVENANCE.txt now records the capture host's inventory and the amended re-capture recipe, because the recipe at the top of that file hard-errors for these eight by design. Capturing on a host with west installed and the pin dropped would freeze an answer that only replays on hosts that also have west, reintroducing exactly the bug #324 closed.
…in the native-sim e2e The autouse _scrub_sdk_discovery_env fixture isolated ALP_SDK_ROOT, SOURCE_DATE_EPOCH, HOME and USERPROFILE but left ZEPHYR_BASE inherited from the developer's shell, so one more discovery input could differ between a local run and CI. It is scrubbed now. test_native_sim_e2e's _zephyr_base() read os.environ directly, which would have defeated that scrub; it reads REAL_ENVIRON instead, matching the idiom the same file already uses for the real west build subprocess env. Verified with and without a hostile ZEPHYR_BASE exported: byte-identical counts either way across the doctor, kconfig and native-sim suites. This is NOT a fix for #297, which stays open. That issue's diagnosis -- that an ambient ZEPHYR_BASE makes test_west_resolved_reproduces_and_closes_tan_ cli_123 flaky -- does not survive measurement: find_workspace_venv's upward .venv walk resolves before the ZEPHYR_BASE read is ever reached, and the test plants its .venv at the subprocess's own cwd. With a hostile value set, that test gives 5 passed / 128 deselected, identical to the unset baseline. The original Windows-side failure remains unreproduced and needs a fresh repro on Windows.
…ref (#213) client_payload.sdk_ref was read into a step output and echoed via ::notice::, which is job-log-only -- it never reached the Actions API display_title field alp-sdk polls to correlate the run it triggered. A top-level run-name key surfaces it. The non-dispatch arm is an empty string, not a formatted event name: GitHub falls back to the event-specific default title when run-name is empty or whitespace, so push and pull_request runs keep showing their commit message or PR title. Naming the event there instead would have overwritten that default on every such run -- a regression traded for the fix. Same idiom this file already uses one line away for ref:. Inert until it reaches the default branch.
alpCaner
added
bug
…numbers (#224) The gate landed keyed on (file, ABSOLUTE LINE), which reds the build on any unrelated edit that shifts a declared site. It happened immediately: dev's fc88ca1 added +69/-18 lines to bootstrap_cmd.py, moving f"bootstrap.{code}" from :1522 to :1532, and both the `python` and `seam1 -- plan-shape parity` CI jobs went red on the PR's merge commit while the branch itself was green. Re-pinning the number was the wrong fix. A gate that must be hand-re-pinned after every unrelated edit trains reviewers to update the pin mechanically without reading what changed, which is how a gate stops gating -- the tan-cli#275 lesson this file already cites. Keys are now (file, enclosing qualname). Line numbers stay in the error messages, where a human needs them, and appear in no hand-maintained key. Proved by measurement: shift bootstrap_cmd.py, gate still passes; plant an unregistered code, gate still fails naming it. Re-keying opened a smaller hole, found by review and reproduced: a SECOND template inside an already-declared function collapses onto one key, leaving only the scalar EXPECTED_TEMPLATE_COUNT -- whose own failure message says to bump the count. So each declaration now pins how many sites it covers, and a mismatch fails naming the qualname, both counts and every matched file:line. _FORWARDER_SUFFIXES, the third bucket with the same many-to-one shape, gets the same discipline. _check_site_counts was a closure and could not be tested. This file states its own bar -- an assertion nobody has ever watched fail is not proven to fire -- and met it for its two older assertions. It is extracted and tested now, including the _ACKNOWLEDGED_CEILINGS half that is empty in production and had never had a real value driven through it. One hole is documented rather than closed. Resolving an f-string statically cannot tell a substituted name bound by the enclosing function's parameter list from one rebound by a comprehension target of the same spelling, so a 1-for-1 swap at a declared key needs no count bump at all and stays green. That is a limit, not a missing pin; chasing it means shadowing heuristics that false-positive on ordinary Python. The module docstring records it as an acknowledged ceiling with the verbatim repro and names the real backstop: contract/issue-codes.json is read directly by release.yml when building the published envelope-contract.json, and alp-sdk-vscode fails open on a code it does not recognise, so the cost is a silently-ignored issue, not a crash. Also drops three line-number comments that had themselves rotted (2072, 2061, 2326 -> 2099, 2088, 2363) rather than re-measuring them, in the file whose whole subject is that line identity decays.
…nstead of asserting it Milestone v0.6.0's goal is, verbatim, "Full command-surface parity with the v0.4.1 oracle: model, new-som, monitor, faultdecode, the introspection set, renode, and the seven entirely-unported verbs." Not one of those verbs appeared in this file's CASES, which covered --version, a bogus command, a bare invocation, debug-config, presets, clean and build --plan. The milestone's central claim had no instrument that could read it. Nine case-functions (17 node IDs) now cover every named verb, each established by RUNNING target/debug/tan rather than by reading crates/. Matching, as live parity cases: explain (overview, unknown-template, unknown-target), image, renode -- full envelopes byte-identical. Diverging, each as a documented known-divergence case that pins the divergence on BOTH sides so it fails if silently fixed or worsened: model, new-som, monitor, faultdecode, size, run, and all seven deferred verbs. Several are structural, not cosmetic. faultdecode's exit codes coincide at 2 while the envelopes differ entirely -- the port re-implemented it as ARMv8-M register arithmetic that reads no SDK, so it answers cli.parse-error where the oracle answers faultdecode.failed. new-som's Click command declares no --format option at all, so --format json is a usage error rather than an envelope. size's divergence is permanent: two runtimes rendering the same ENOENT, Rust's `(No such file or directory (os error 2))` against Python's `([Errno 2] No such file or directory: '<path>')`. Two harness defects had to be closed for any of this to mean anything. The support-bundle case first encoded this host's tool inventory, gaining a third issue under a stripped PATH -- tan-cli#313 and tan-cli#324 reintroduced in the PR that fixed them. These cases spawn both binaries live every run, so unlike the frozen-replay cases they can pin PATH SYMMETRICALLY, and now do. empty_tool_inventory also seeds a real `which`: with a literally empty PATH the oracle's probe cannot resolve `which` itself and reports every tool missing because it could not look, which is an artefact, not an answer. It raises now rather than silently returning to that. rust_binary() preferred target/release over target/debug. On a tree with a stale release build that silently measured against the wrong oracle -- forced over the other four parity files it gave "108 failed, 8 passed", red but reading as port bugs, with eight cases passing against the wrong binary. It picks the most recently built profile now, refuses an mtime tie rather than breaking it silently, and a session-scoped fixture in the new conftest.py asserts the resolved binary's --version for every file under tests/parity. Wrongness fails loudly; absence stays a skip, which is each file's own gate to decide (missing_for_live's rule, tan-cli#272). Resolution happens in the fixture body, not at conftest import, so a raise cannot abort collection for the whole suite.
# Conflicts: # crates/tan-cli/src/commands/bootstrap/steps.rs
…bootstrap warn count Merging dev put the tan-cli#224 emit-site gate in one tree with two commits that landed while this branch was in flight, and it caught both. fcb2153 (tan-cli#325, `fix(init,generate): confine writes to the project after symlink resolution`) added `generate.write-escapes-project` -- raised as a GenerateError with ExitCode.WRITE_FAILURE when resolve_confined finds a target's output path resolving outside the project root -- and registered it nowhere. That is ungated on both sides of the seam at once: this repo's registry-driven checks never see it, and the published envelope-contract.json is built from that same registry, so alp-sdk-vscode cannot see it either. Registered `reserved`/`consumer: none`, which costs nothing since a reserved code may still be renamed freely. It is unregistered on dev right now, not only here -- this registration fixes it for everyone at merge. 518ac8c (tan-cli#334) split the single `zephyr-base-incompatible` warn into a found/else pair so the message can name the evidence, taking that helper from 16 call sites to 17. One code, two sites, already registered. Checked before bumping, which is the entire point of pinning the count: it forced the look rather than allowing a reflexive increment.
15 tasks
Contributor
Author
|
Superseded by #352, which carries this work rebased onto Carried over intact#224, #308, #309, #311, #312, #313, #324, #213, plus #180 / #179 / #139 closed One commit dropped#318's Two defects found where your work met the merged #336git merged your #308 and the already-merged #336 cleanly — textually. 1. #308 was silently cancelled. #308 fills CI would have been green. Every test in 2. #309's guard message was deleted. #336's re-wording block sat after The pop now keys on the assembled WindowsFive of #308's unit tests hardcoded POSIX literals against values the code Also: your Where it is now#352 also carries #260/#257 (the seven deferred verbs are real commands), Thanks for #331 — the triage in it is what made the batch tractable. |
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
1 participant
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.
Ten issues from the v0.6.0 milestone triage, each fixed, reviewed and verified against the current tree rather than against its own issue text.
Fixed
#224 — the emit-site gate could not see codes built by a prefixing helper.
release-blocker. The Rust gate (crates/tan-cli/tests/contract.rs:735) was fixed butcrates/ships to nobody, so on the shipping surface no source-to-registry scan existed at all. Newpython/tests/gates/test_every_issue_code_is_registered.pycatches literal emit sites and f-string assembly through a prefixing helper (bootstrap_cmd.py,debug_config_cmd.py,doctor_cmd.py). It found 146 unregistered issue codes, all now registeredreservedincontract/issue-codes.json. The gate carries a self-test proving it rejects a deliberately unregistered code — a gate that cannot fail is not a gate (tan-cli#275).#309 — an
os: zephyrslice that never loaded Zephyr was reportedok. ACMakeLists.txtthat never callsfind_package(Zephyr ...)still configures and links underwest build, so exit code 0 alone was letting a plain host binary pass as firmware. Ports the oracle'szephyr_boilerplate_loaded/dir_shows_zephyrintopython/tan/commands/build/manifest.py, applied inexecute.py's status assembly. Handles the--sysbuildcase, where only the nested per-image build carries the signal.#308 — a stale ambient
$ZEPHYR_BASEbeat the workspace tan had just resolved.build_cmd.pypassedgap_fillers=()with a literal# NOT YET PORTED. Newpython/tan/core/zephyr_env.pyderivesZEPHYR_BASE/EXTRA_ZEPHYR_MODULESfrom the resolvedwest_workspace_dir.EXTRA_ZEPHYR_MODULESkeeps its;separator on every platform via the existingsep_for_key, and an inheritedPYTHONPATHis still extended rather than replaced.#311 — an ELF artefact could have its headers written into on-die MRAM.
plan_alif_mram_jlinkemittedloadbinunconditionally, with nois_raw_binguard, unlike its siblingplan_swd_probe. Now raisesFlashPlanErrornaming the artefact and pointing atzephyr.bin. Deliberately a refusal, not aloadfilefallback — that would silently ignoreslot0_load_address. Latent today (no in-treeboard.yamlsets that field), fixed before it isn't.#312 — the DPIDR preflight blamed the wiring for a probe that was still re-enumerating. A banner carrying a different DP ID and a banner carrying none at all both got "Check the probe selection (
flash_args.jlink_serial) and the wiring." Now distinguished; the second advises a retry. Refusal behaviour is unchanged — both still refuse to write MRAM.#318 — a required check was red on the release branch.
ensure_venv_reuses_a_venv_with_usable_pipwrote its shim inline and spawned it; undercargo test's parallel threads a sibling fork could hold the write fd, making the spawn fail ETXTBSY. Addswait_until_spawnableat both inline shim sites, exactly as204c3a0did forexit_code_shim. Measured 20/20 green against a 1-in-20 pre-fix failure rate.#313 and #324 — frozen parity fixtures encoded the capture host's tool inventory. Eight cases branch on a
which()probe, so their frozen answers recorded which tools the capture host had. Three yocto_wic cases failed wherever neitherbmaptoolnorddexisted; threewest_forwardcases failed whereverwestdid. Neither was a port gap — under a sanitised environment the oracle and the port emit byte-identical envelopes. The replay now pins the Python side's PATH to the recorded inventory.PROVENANCE.txtdocuments the inventory and the amended re-capture recipe, because the original recipe now hard-errors for these eight by design.#213 —
parity.ymlruns were not correlatable by alp-sdk. Addsrun-namesurfacingclient_payload.sdk_refto the Actions APIdisplay_title. The non-dispatch arm is an empty string so GitHub's default title still renders on push and pull_request runs.Also closed as already-fixed, with evidence on each: #180, #179, #139.
Not fixed, and why
#297 stays open. Its diagnosis — that an ambient
ZEPHYR_BASEmakestest_west_resolved_reproduces_and_closes_tan_cli_123flaky — is wrong.find_workspace_venv's upward.venvwalk (python/tan/core/venv.py:66-72) resolves before theZEPHYR_BASEread at:74is ever reached, and the test plants its.venvat the subprocess's own cwd. Measured with a hostileZEPHYR_BASE:5 passed, 128 deselected, identical to the unset baseline. The original Windows failure remains unreproduced on Linux and needs a fresh Windows-side repro.A genuine
ZEPHYR_BASEleak did turn up elsewhere and is fixed here:conftest.py's autouse_scrub_sdk_discovery_envnow scrubs it, andtest_native_sim_e2e.pyreadsREAL_ENVIRONinstead ofos.environ. That is not #297's cause and is not claimed as one.The
crates/change#318 touches the frozen tree. That is deliberate and narrow: the test is a required check (
test (ubuntu-latest)) that is red today while0.5.0-rc3is being promoted, the change is test-hermeticity with zero production delta, and204c3a0set the precedent in the same file for the same root cause. The race does not port —cargo testruns tests in parallel threads, while the Python suite is serial (no xdist, noaddoptsbeyondpythonpath), so there is no concurrent-fork window on the Python side and no counterpart fix is needed.Filed while triaging
yocto-userspacedebug draft'smiDebuggerServerAddressis a<host>:<port>placeholder nothing resolves, and alp-sdk-vscode'spreLaunchTaskFordeliberately excludes that target kind, so nothing starts the gdbserver either. F5 fails at connect, silently.Gates
python -m pytest tests -qfrompython/— see the run in the PR checks; zero failures is the bar, not a countcargo fmt --all --check— exit 0, no outputcargo clippy --all-targets --workspace -- -D warnings— exit 0, no warningscargo build --all-targets— cleancargo test --locked— exit 0, 1145 passed / 0 failed across all 11 test binaries,frozen_issue_codesgreen against the 146 new registry entriesNote for the release
contract/issue-codes.jsongains 146reservedentries and zero removals. It ships as a release asset, and this repo writes CHANGELOG sections at release time rather than per PR, so the next release notes should name it.Closes #224
Closes #308
Closes #309
Closes #311
Closes #312
Closes #313
Closes #318
Closes #324
Closes #213