fix(ebuild): ask ninja for the Windows test binary

[thompsondev]

Summary

On Windows, native type: test targets are linked as <name>.exe, but ebuild test asked Ninja to build <name> and then looked for that unsuffixed path. Ninja reported an unknown target; the runner reported "built, but no binary".

A second Windows bug aborted pytest collection when gcc was not installed: a skipif called subprocess.run(["gcc", "--version"]), which raises FileNotFoundError instead of returning non-zero.

Review (four rounds, see the comments below) found two more call sites with the same unsuffixed-path bug — ebuild package and the flash/RAM footprint report ebuild build prints — plus several process issues (stale PR description, self-verified completed tasks, a duplicated test probe, a fragile new test, an imprecise root-cause writeup, an asymmetric evidence ledger). All of those are now addressed on this branch; this description reflects the branch as it stands today, not as originally opened.

Type of Change

• fix — Bug fix
• test — Add or fix tests
• docs — Documentation updated with the behavior change

Changes

• Add executable_output_path() in ebuild/build/ninja_backend.py — the single source of truth for "where does the linked binary land", .exe-suffixed on Windows because the Ninja edge in _write_ninja already names it that way via _exe_suffix().
• Four call sites, across three consumers, now go through it — all previously re-deriving the naming rule independently and dropping the suffix on Windows:
  • _run_native_tests (ebuild test, ebuild/cli/commands.py:2624 and :2633) — both the Ninja target list and the binary that is executed.
  • package command (ebuild package, commands.py:2534) — the artifact lookup; previously reported "No built artifact" after a successful Windows build.
  • _report_footprint (ebuild build's flash/RAM summary, commands.py:516) — previously failed silently on Windows (no report, no log line); now also logs at debug level when there's nothing to measure, matching the function's other early exits.
• Replace the collection-time gcc skipif in tests/ebuild/test_build_dir_resolution.py with shutil.which plus OSError handling; the probe (gcc_is_missing()) is deduplicated into tests/support.py and shared with tests/unit/test_footprint.py (not tests/conftest.py — that file has no fixtures/hooks of its own, so importing a helper from it fought the plugin's own contract).
• Regression tests for all three fixed consumers force _exe_suffix() to .exe via monkeypatch rather than branching on sys.platform, so each one exercises the Windows path — and fails against the pre-fix code — on any host the suite runs on. Verified directly: each test reverted individually against its call site's pre-fix code and confirmed to fail; restored and confirmed to pass. The footprint test also chdirs into tmp_path so a real eos.yaml/board.yaml at the process cwd can't change what it measures.
• Document the ebuild test, package and footprint fixes in CHANGELOG (reworded to the actual internal cause — the edge already carried the suffix, the consumers rebuilt the path without it — rather than "gcc appends the suffix"); document the ebuild test fix in README too. No docs/compatibility.md change: an earlier edit there nets out to zero diff against master (it removed exactly the row an earlier version of this PR had added), so that file isn't part of this diff.
• TASKS.md: T-002 (this PR's original subject), T-003 (package) and T-004 (footprint) are tracked in the Active table at review, not Completed — the evidence for all three is self-reported by the implementer (me/Claude) and is explicitly marked pending independent review, per this repo's own reviewer role boundary ("if you implemented it, you do not approve it"). T-005 (backlog: move the helper out of the Ninja-specific backend module) is filed at todo.

Testing

• New tests added for new functionality
• pytest run on Windows (Python 3.12) — run on a real Windows host, but Python version not confirmed against 3.12 specifically
• Unit tests pass (ctest --test-dir build --output-on-failure) — not applicable; this is the Python suite
• All existing tests pass, on this host — see below; the project's own CI has not run on this head (queued, pending maintainer approval of fork-workflow runs — outside contributor policy, same as ebuild#109)

Command: pytest

Result, single run at this head, this Windows host: 560 passed, 6 skipped, exit code 0. This supersedes every other count quoted anywhere in this PR's history — an original 1 failed, 557 passed figure and two intermediate 559/560-passed figures each taken mid-fix — none of which were reproducible against this head, and TASKS.md now carries only this one.

No stub or monkeypatch runs a real subprocess/ninja/gcc/efwtool invocation on Windows; the _exe_suffix()-forcing tests are a proxy for that (they exercise the path-construction logic, not the OS-level linking/execution behavior), and are the best available substitute for an actual Windows CI leg, which has not run. Independent review on Linux (see comments below) reproduced all four regression tests failing against the unfixed code and passing against the fix, and found zero remaining unsuffixed-path call sites in ebuild/.

Pre-Submission Checklist

• Code compiles without warnings (-Wall -Wextra -Werror for C) — not applicable
• All existing tests pass, on this host — project's own CI has not run on this head (see Testing)
• New tests added for new functionality
• Documentation updated
• Commit messages follow <type>(<scope>): <description>
• Branch is rebased on latest master — not confirmed; diff applies cleanly to origin/master and GitHub reports mergeable: MERGEABLE, so there's no known conflict, but this hasn't been checked directly

Related Issues

Addresses T-002 (Windows Ninja test-target path parsing), T-003 (ebuild package unsuffixed .exe path) and T-004 (footprint report unsuffixed .exe path, found during review). All three are tracked at review status in TASKS.md's Active table, not Completed — self-implemented fixes await independent sign-off before that table move, per this repo's reviewer-role rule. T-005 (helper's module placement, non-blocking) is filed as backlog at todo.

Additional Notes

Superseded: earlier versions of this description said the package fix "was left out of this PR so the test-runner fix stays reviewable on its own" and listed T-003 as left open, attributed the Windows bug to "gcc appends the suffix", and quoted a suite result no longer reproducible at this head. All of that is corrected above; see the review comments below for the four rounds that found each issue. The footprint-report fix (T-004) was not part of the original PR at all; it surfaced during the third review round.

[srpatcha]

Review — ebuild#110 "fix(ebuild): ask ninja for the Windows test binary"

head: a65206b author: thompsondev ci: none reported

Verdict: The fix is right and minimal — one helper, one source of truth, both call sites
converted. I reproduced the original bug on Linux by forcing the suffix and confirmed the fix
resolves it. The PR body is unusually honest: unchecked boxes stay unchecked, the failing test
is named, and the deferred half is filed as T-003 in three places. The one real gap is that
the new regression test is platform-gated the same way the code is, so on a POSIX runner it
passes against the unfixed code too — verified.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	tests/unit/test_golden_path_commands.py:171-213	test_native_runner_asks_ninja_for_the_linked_binary computes suffix = ".exe" if sys.platform == "win32" else "" at :193 — the same gate _exe_suffix() uses — then asserts str(build / f"t_smoke{suffix}") in runs[0]. On Linux that reduces to asserting the unsuffixed path, which is exactly what the buggy code produced. Verified: applied only the PR's test files onto origin/master, leaving commands.py:2616 at str(build_path / t.name), and ran the test — it passes against the unfixed code. A Windows-only bug's regression test does not exercise the Windows path on any runner the project has, so it will not catch the same mistake in package (finding 2) or the next call site.	Force the behaviour instead of reading the platform: monkeypatch.setattr(ninja_backend, "_exe_suffix", lambda: ".exe"), then assert the ninja argument ends with t_smoke.exe. I ran precisely that probe against both trees: master: asked for …/b/t_smoke edge t_smoke.exe -> _run_native_tests raised SystemExit: 1 this PR: asked for …/b/t_smoke.exe edge t_smoke.exe -> no failure That version fails on master and passes here, on Linux, which is what a regression test for this bug has to do. Keep the existing test as the POSIX case if you like, but the suffix assertion needs to be independent of the host.
2	Low	ebuild/cli/commands.py:2534	ebuild package still does artifact = Path(build_dir) / binaries[0].name, so on Windows it reports "No built artifact at _build\app. Run 'ebuild build' first." after a build that succeeded. Deferring it is defensible and the disclosure is genuinely good — T-003 in TASKS.md, a row in docs/compatibility.md's Known Limitations table, and a note in the body. But the reason for deferral was reviewability, and after this PR the fix is one call: executable_output_path(Path(build_dir), binaries[0].name), with the helper already imported in this module at :28-32. Grepped the package: commands.py:2534 is the only remaining unsuffixed executable path in ebuild/. That leaves §9's build → test → package sequence broken at the last step on Windows, one line short.	Either take the one-liner here, or say in the body that the deferral is deliberate now that it is a one-liner — the current wording ("left out so the test-runner fix stays reviewable on its own") was written when it was not. Whichever way, finding 1's suffix-forcing test should cover package too, or T-003 gets the same untestable fix.
3	Medium	checks.txt (empty); GitHub API	No CI has run on this head. Queried it: actions/runs?head_sha=a65206b4… returns 4 runs — Simulation Test, CI — ebuild, Auto Assign PR, CodeQL — every one status=completed, conclusion=action_required, and commits/a65206b4…/status is {"state":"pending","count":0}. The runs are queued behind fork-workflow approval. So the body's 1 failed, 557 passed, 5 skipped on Windows/Python 3.12 is unreproduced, and there is no Linux result either — which matters here, because the change is platform-conditional and the two platforms take different branches.	Maintainer action, not the author's: approve the workflow runs. Same gap ebuild#103 exists to close.
4	Low	tests/ebuild/test_build_dir_resolution.py:246-259, 277-280	_gcc_is_missing() is still evaluated at import time, as the argument to skipif. That is fine — it is now wrapped in try/except OSError, which is the actual bug fix — but the function only tests the OSError branch (test_gcc_probe_does_not_raise_when_gcc_cannot_start monkeypatches which to return a path that cannot start). The shutil.which(...) is None branch, which is the common case on a bare Windows host, has no test.	One line: a second case monkeypatching shutil.which to lambda name: None and asserting _gcc_is_missing() is True. Worth noting the change is otherwise exactly right — it brings this file into line with test_ninja_backend.py:132 and test_integration_initramfs_security.py:38, which already use shutil.which in their skipif. Those two were checked; they cannot raise. This was the only offender.

Architecture conformance

Master design §9 (eBuild as the central developer control plane; ebuild build / test /
release as one flow), §9.2 (SDK design rules — "one source of truth for CLI, VS Code and
EoStudio", "actionable diagnostics with remediation guidance"), §21 Tier 1 – Foundation,
§28 (status and claims policy). .ai/tooling.md: "Exit non-zero on failure, always."
Conforms, and it is the §9.2 fix rather than a workaround.

executable_output_path() is the right shape for this: the Ninja edge at
ninja_backend.py:251-254 and the runner at commands.py:2616, 2632 now derive the path from
one function instead of each spelling the convention out. That is "one source of truth"
applied at the level it actually bites — before this, the generator and the consumer of the
generated file each knew the naming rule independently, and they disagreed. §9.2's other rule,
actionable diagnostics, is why the old symptom was expensive: built, but no binary at _build\t_smoke names a path that was never going to exist, and points the reader at the
linker rather than at the caller.

Nothing here points up a tier. ebuild/cli importing ebuild/build is within-package and
downward. §28 is worth a positive note: the body's checklist leaves "All existing tests pass"
unchecked with "not claimed", names the pre-existing failure and its cause, and lists the
skips. That is the evidence policy being followed rather than performed, and it is rarer than
it should be.

Verified by running, on origin/master + this patch and on master alone:

git apply                                        -> clean

with ninja_backend._exe_suffix() forced to ".exe" (simulating Windows):
  master   ninja asked for  …/b/t_smoke        build.ninja edge: t_smoke.exe
           -> _run_native_tests raised SystemExit: 1      (the reported bug)
  this PR  ninja asked for  …/b/t_smoke.exe    build.ninja edge: t_smoke.exe
           -> no failure                                   (the fix)

master + this PR's test files only (source fix absent), on Linux:
  test_native_runner_asks_ninja_for_the_linked_binary -> PASSES   (finding 1)

on this head, Linux:
  test_native_runner_asks_ninja_for_the_linked_binary -> PASS
  test_gcc_probe_does_not_raise_when_gcc_cannot_start -> PASS

grep for remaining unsuffixed executable paths in ebuild/:
  commands.py:2534 only  (finding 2 / T-003)

Proposed changes

1. Make the regression test force the suffix rather than read sys.platform (finding 1). The
   probe above is the shape; it is a two-line change and it converts a test that cannot fail
   into one that does.
2. Take the one-line package fix, or restate why it is still deferred (finding 2).
3. Add the which is None case (finding 4).

No fix PR opened: findings 1, 2 and 4 all belong on this branch, and the brief forbids
touching a branch belonging to an open PR. commands.py:2534 does exist on origin/master,
but a separate PR for one line that this branch is already about would be a duplicate.

Not checked

• No CI has run on this head (finding 3). Nothing here has been verified by the project's
  own pipeline; everything above is my own execution in a scratch tree.
• Nothing was run on Windows. There is no Windows host available here, so every claim
  about Windows behaviour comes from forcing _exe_suffix() to return ".exe" on Linux. That
  exercises the path-construction logic, which is the whole of this change, but it does not
  test subprocess/ninja behaviour with real Windows paths, drive letters, or quoting.
• pytest is not installed in this environment. The two new tests and the probe were run by
  importing each module with a minimal pytest stand-in (mark as identity decorators,
  monkeypatch as a record-and-undo shim, tmp_path as a real temp dir) and calling the
  functions directly. The body's full-suite figure (1 failed, 557 passed, 5 skipped) was
  not reproduced, and neither was the named pre-existing failure
  tests/unit/test_footprint.py::TestMeasure::test_measures_a_real_binary.
• No real ninja invocation. subprocess.run was stubbed in every run above, so ninja was
  never asked to build anything — what was checked is the argv it would receive.
• ebuild package was not executed on any platform; finding 2 is from reading :2534 and the
  grep above.
• The local ebuild clone was left alone — the sync step reported it dirty (4 files, on
  branch v90). All inspection was via git archive origin/master into scratch directories,
  touching neither the working tree nor the index. origin/master was confirmed current
  against git ls-remote first.
• mergeStateStatus: BLOCKED, reviewDecision: REVIEW_REQUIRED, and the body's own unchecked
  "Branch is rebased on latest master" — not investigated.

---

Automated architecture review of a65206b40b70 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[srpatcha]

Review — ebuild#110 "fix(ebuild): ask ninja for the Windows test binary"

head: c7d83d8 author: thompsondev ci: none reported (see CI)

Verdict: The fix is the right shape — one helper in the backend, consumed by the CLI — and the whole suite is green here (565 passed). But there are three places that build an executable path from a target name and this PR fixes two; the third silently suppresses the flash/RAM report on Windows. The PR body also describes a different change than the diff contains.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	ebuild/cli/commands.py:516	Third site, still unsuffixed: _report_footprint does artifact = build_path / binaries[0].name and then if not artifact.is_file(): return. On Windows the linked file is <name>.exe, so ebuild build never prints the flash/RAM report. It is the silent one of the three — the other two early exits in this function log at debug level (:522, :530), this one returns with no diagnostic, so a developer cannot tell the report is missing rather than not applicable. §9.2 lists "Per-target flash/RAM reports" as an SDK design rule and §25.2 puts memory/flash reporting in MLP scope; the function's own docstring says "A developer who has to run size themselves … is not being told". Verified by execution (see CI section): with _exe_suffix() forced to .exe, the artifact on disk is app.exe, the lookup asks for app, and the function prints nothing even with /usr/bin/size present.	artifact = executable_output_path(build_path, binaries[0].name). Confirmed to restore the report (Flash: 1.7 KB / RAM: 4.6 KB / Ready to flash.) with no other change. Add a test that forces _exe_suffix() to .exe, mirroring the two this PR already wrote — that pattern is exactly what makes this provable on a Linux host. Consider changing the bare return to a log.debug at the same time, so the next instance of this is visible.
2	Medium	pr.json body vs the diff	The body describes a smaller change than the diff contains, in three places. "Additional Notes": "ebuild package has the same Windows suffix mismatch. It was left out of this PR" — but the diff does fix it at commands.py:2533, the CHANGELOG entry documents it as fixed, and TASKS.md:61 moves T-003 into Completed. "Related Issues" still says "Leaves T-003". And "Changes" claims the fix is documented in "README, CHANGELOG, and docs/compatibility.md" — docs/compatibility.md is not in the diff (9 files, verified against origin/master). A reviewer trusting the notes skips the package change and its ripple, which rewrote two test_package_efw.py fixtures.	Rewrite Additional Notes / Related Issues to match: T-003 is fixed here, not deferred. Either add the docs/compatibility.md paragraph or drop the claim.
3	Medium	pr.json body "Testing" vs TASKS.md:61	Two contradictory suite results in one PR. Body: "1 failed, 557 passed, 5 skipped, exit code 1", with the failure attributed to test_footprint.py::TestMeasure::test_measures_a_real_binary. TASKS.md:61, added by this same diff: "Full suite run on this Windows host: 559 passed, 6 skipped, 0 failed." Both cannot describe head c7d83d8, and the body's named failure is the very test this diff adds a skipif for — so the body appears to predate its own change. Per the brief, a claim the PR itself contradicts is the finding.	Keep one figure, from one run, on this head, and name the host. The body's checklist line "All existing tests pass — not claimed" is good practice and worth keeping; it is the numbers above it that disagree.
4	Low	tests/unit/test_footprint.py:61-64 and :71-74	Two mechanisms guard the same condition and one is unreachable. The new @pytest.mark.skipif(shutil.which("gcc") is None, …) means the body only runs when gcc is on PATH, so the inner except FileNotFoundError: pytest.skip(…) can only fire if gcc disappears between collection and execution. Separately, this file's probe is the bare shutil.which("gcc") is None while tests/ebuild/test_build_dir_resolution.py:246 now has the hardened _gcc_is_missing() — two probes for one question, one of them the version this PR exists to fix.	Move _gcc_is_missing() into tests/conftest.py and use it in both files, then drop the inner try/except. One probe, one place, and the next file that needs it inherits the hardened version.

CI

checks.txt is empty and no checks are reported on fix/windows-ninja-test-target-paths. Same cause as ebuild#109, and it is not a broken workflow:

gh run list   # CI — ebuild / CodeQL / Simulation Test, event=pull_request
              # completed  action_required  0s

action_required means the runs are queued awaiting maintainer approval under this repo's outside-contributor Actions policy. ci.yml triggers on pull_request: branches: [master, main] and this PR's base is master, so the trigger is correct. A maintainer needs to click "Approve and run workflows"; nothing the author can do from the PR will produce evidence. Until then this PR's Windows claims — which are the entire point of it — cannot be checked by CI on any Windows runner.

Reproduced locally at this head (fresh worktree, venv with pytest, click, pyyaml, ninja; gcc and /usr/bin/size present):

pytest -q                                        # 565 passed in 3.57s
pytest -q -k "native_runner_asks_ninja or gcc_probe or windows_suffixed_artifact"
                                                 # 4 passed, 561 deselected

So the suite is fully green on Linux, and both _exe_suffix()-forcing tests do exercise the Windows path on a POSIX host as the docstrings claim. Neither number quoted in the PR reproduces here, which is expected across hosts — but it does mean the body's "1 failed" is not a property of the code.

Finding 1 verified by execution, both directions, with _exe_suffix() forced to .exe:

linked artifact on disk:            app.exe          (exists)
what _report_footprint looks for:   app              (does not exist)
_report_footprint output:           <nothing>
with `artifact = executable_output_path(build_path, binaries[0].name)`:
  Flash:     1.7 KB
  RAM  :     4.6 KB
  Ready to flash.

I did not open a fix PR for finding 1. It is a one-line change inside this PR's own subject area, and an autofix/ branch off master would have to add the same executable_output_path import block this branch adds, colliding with it. Per the brief, a duplicate fix is worse than none — it belongs in this PR.

Architecture conformance

Conforms, with one deviation.

• §9.2, "One source of truth for CLI, VS Code and EoStudio" — conforms, and this is the good part of the PR. The path rule now lives once, in the build backend (ninja_backend.executable_output_path), and the CLI consumes it. The dependency direction is right: ebuild/cli depends on ebuild/build, never the reverse. Exposing it as a public helper rather than leaving _exe_suffix() private is what let the writer and the two readers converge.
• §9.2, "Per-target flash/RAM reports" — deviates, via finding 1. Two of three consumers were converted.
• §21 tier placement — correct. Tier 1 ebuild, developer-platform work, no cross-tier reach. §5.1 is not engaged; no dependency direction changes and nothing points up a tier.
• .ai/reviewer.md, weakened checks — checked, and nothing here qualifies. The skipif added to test_footprint.py replaces a probe that aborted collection, and gcc is present on ubuntu-latest, so the assertion still runs on the leg that can run it. That is an environment guard, not a disabled test. Finding 4 is about the duplication, not the skip.

Proposed changes

1. Finding 1 — one line in _report_footprint, plus the .exe-forcing test. This is the change that makes the PR's own claim ("so the Ninja edge and ebuild test use the same binary path") true of every consumer rather than two of three.
2. Finding 4 — hoist _gcc_is_missing() to tests/conftest.py, use it in both files, drop the now-dead inner handler.
3. Findings 2 and 3 — reconcile the body with the diff, and quote one suite result from one run on this head.
4. Nothing else needs to move. The helper, its placement, and the two forced-suffix tests are the right design and I would not change them.

Not checked

• No Windows verification at all. Everything above is Linux/CPython 3.12. The PR's entire subject is Windows behaviour, and the _exe_suffix()-forcing tests are a good proxy but they are a proxy: they never invoke a real gcc that appends .exe, never call a real ninja against a real edge with a suffixed output, and never exercise a path with a backslash separator. CI would cover this, and CI has not run.
• ebuild package end to end was not run against a real efwtool — the test monkeypatches shutil.which to a stub, so the .exe lookup is verified but the packaging is not.
• macOS was not exercised. _exe_suffix() returns "" for anything that is not win32, which is right for Darwin, but no run confirms it.
• ruff/lint not run — not installed here.
• The TASKS.md T-003 evidence paragraph asserts that fixing package broke existing test_package_efw.py fixtures on a real Windows host and that they were repaired. The repaired fixtures pass here, but the original Windows failure that motivated the change was not reproduced.
• docs/compatibility.md content was read but not assessed for whether it should carry this note; finding 2 only reports that the body claims a change to it that is not present.

---

Automated architecture review of c7d83d86ef99 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[srpatcha]

Review — ebuild#110 "fix(ebuild): ask ninja for the Windows test binary"

head: 26a767f author: thompsondev ci: none reported (checks.txt empty; mergeStateStatus: BLOCKED, reviewDecision: REVIEW_REQUIRED)

Verdict: The code fix is right and conforms — one executable_output_path() helper replaces three duplicated build_dir / name lookups so ebuild test, ebuild package and the footprint report all name the binary the Ninja edge actually links. The problems are in the claims around it: the PR body describes a smaller change than the diff contains, and TASKS.md records two tasks as Completed / "Verified by: self" in the same PR that implements them.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	pr.json body vs. ebuild/cli/commands.py:2531, 516	The PR body describes a different change than the diff. It says the package fix "was left out of this PR so the test-runner fix stays reviewable on its own" and "Leaves T-003 (ebuild package unsuffixed .exe path)" — but the diff fixes package (commands.py:2531) and _report_footprint (commands.py:516), and TASKS.md marks both done. It also claims documentation in docs/compatibility.md, which is not in files.txt. A reviewer reading the body reviews the wrong PR.	Rewrite Summary / Changes / Related Issues against the head commit: three call sites fixed, T-002/T-003/T-004 all addressed, and drop the docs/compatibility.md claim or add the file.
2	Medium	pr.json body "Testing" vs. TASKS.md:68, TASKS.md:69	Three mutually exclusive suite results are asserted for one PR: body "1 failed, 557 passed, 5 skipped, exit code 1"; T-003 "559 passed, 6 skipped, 0 failed"; T-004 "560 passed, 6 skipped, 0 failed". The body further names the remaining failure as test_footprint.py::TestMeasure::test_measures_a_real_binary — the very test this diff puts a gcc_is_missing() skipif on (tests/unit/test_footprint.py:61), so at head it skips rather than fails. None of the three runs can be matched to 26a767f4. Per brief §5, the unsupported claim is itself the finding.	Quote one pytest run taken at the head commit, with its exit code, and delete the two superseded counts.
3	Medium	TASKS.md:68-69	T-003 and T-004 are moved to Completed with Verified by: self (see PR #110 review, finding 2) by the same PR that writes the code. .ai/reviewer.md makes this the one structural role boundary: "If you implemented it, you do not approve it." T-001 directly above it records Verified by: independent reviewer, and T-002 — the older task — is still left at review. The ledger now reads as if the newer work has more sign-off than the older.	Leave T-003/T-004 at review in the active table with the evidence text intact; move them to Completed in a follow-up once a reviewer other than the author signs off, matching T-001.
4	Low	tests/unit/test_footprint.py:298	test_looks_up_the_windows_suffixed_artifact calls commands._report_footprint, which calls _selected_board() (reads Path("eos.yaml")) and _board_config() (reads Path("board.yaml")) — both relative to the pytest process CWD, and neither monkeypatched. It passes today only because the repo root ships neither file (git ls-tree origin/master → codecov.yml, mkdocs.yml only). Anyone who runs pytest from a project directory, or who adds a root eos.yaml, changes what this test measures.	Add monkeypatch.chdir(tmp_path), or monkeypatch commands._selected_board and commands._board_config directly, as the assertion only concerns the artifact path.
5	Low	tests/conftest.py:1 (new)	gcc_is_missing() is a plain helper placed in conftest.py and then imported by name (from tests.conftest import gcc_is_missing) from two test modules. It works here — tests/__init__.py exists and pytest.ini sets pythonpath = ., so pytest imports the file as tests.conftest and the explicit import binds the same module object, not a second copy — but conftest.py is pytest's fixture/hook file and importing it from test modules is the one thing its contract asks you not to do. It is also now a suite-wide root conftest holding no fixtures.	Move the function to tests/support.py (or tests/probes.py) and import from there; leave tests/conftest.py out of the PR.
6	Low	ebuild/build/ninja_backend.py:81	executable_output_path() is now the cross-module contract for "where does the linked binary land", consumed from ebuild/cli/commands.py at three sites — but it lives in the Ninja-specific backend module. commands.py already reaches for ninja_command from ebuild.build.dispatch, i.e. the CLI is written as if the backend were dispatchable. Output-path naming is a property of the toolchain/host, not of Ninja.	Not blocking, and not worth churn in this PR: put it on the eBuild backlog to move the helper to a backend-neutral module (ebuild/build/layout.py) and re-export from ninja_backend for compatibility.

Architecture conformance

Conforms. eBuild is Tier 1 — Foundation (§21), and the change stays wholly inside it; nothing is added to eos, eBoot or any product repo, and no dependency points up a tier (§5.1). Within the repo the direction is downward and inward: ebuild/cli → ebuild/build, never the reverse. eBuild remains a build-time control plane and not a runtime dependency (§5.1).

Positively aligned with §9.2, "one source of truth for CLI, VS Code and EoStudio": before this PR the rule "the linked binary is <name> + platform suffix" was encoded once in _write_ninja and then re-derived, wrongly, at three consumer sites. Collapsing them onto one function is the correct shape of the fix, and is why finding 6 is a placement nit rather than an objection.

The behaviour restored also matters for §25.1's golden path — build / test / flash on Windows — and for the §9.2 per-target flash/RAM report, which finding 4's target (_report_footprint) is: on Windows that report silently never appeared. Adding the log.debug on the missing-artifact early return (commands.py:517) brings that path in line with the function's other two exits and satisfies §9.2's "actionable diagnostics".

Proposed changes

Smallest sequence, none of it touching the code fix:

1. Rewrite the PR body — Summary, Changes, Related Issues — to describe three fixed call sites (test, package, footprint) rather than one. Drop docs/compatibility.md from the claim list or add the file (findings 1).
2. Re-run pytest at 26a767f4 and replace all three result counts with that single run and its exit code (finding 2).
3. In TASKS.md, move T-003 and T-004 back into the active table at review, keeping the evidence prose. Nothing else changes (finding 3).
4. In tests/unit/test_footprint.py::TestCLIFootprintReport, add monkeypatch.chdir(tmp_path) (finding 4).
5. git mv tests/conftest.py tests/support.py and update both importers (finding 5).

Steps 4–5 are the only code edits and neither changes product behaviour; the suite should be re-run after them, which also produces the single result step 2 needs.

Not checked

• Nothing was executed. The ebuild working tree is dirty (4 files) and the sync step left it untouched, so 26a767f4 is not in the local object store (git log 26a767f4 → bad object). Every judgement here is read from diff.patch plus origin/master at 6a22e36.
• Therefore all four suite counts in the PR are unverified — I did not reproduce any of them, including the one I recommend keeping.
• The Windows behaviour itself is unverified: no Windows host, no gcc-on-Windows link, so the .exe suffix claim is taken from _exe_suffix()'s sys.platform == "win32" test as read, not observed.
• Findings 4 and 5 were reasoned against origin/master sources (_board_config, _selected_board, pytest.ini, tests/__init__.py) — if the PR branch changed any of those outside this diff, re-check.
• CI is not "pass" and not "fail": checks.txt is empty, so no required check result was available to me at all. mergeStateStatus: BLOCKED is reported but I could not determine what blocks it.
• Whether the branch is rebased on master — the author's own checklist leaves this unticked — was not checked.

---

Automated architecture review of 26a767f4043b — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[srpatcha]

Review — ebuild#110 "fix(ebuild): ask ninja for the Windows test binary"

head: 93fdebd author: thompsondev ci: none run (all three workflows action_required; check-runs: 0; combined status pending)

Verdict: Follow-up, round four. The code is now correct and complete: all four
build_dir / name call sites go through executable_output_path(), and each of the three
product fixes has a regression test that I confirmed fails against the unfixed code on
Linux. Every code finding from the previous three reviews is resolved. One finding is
untouched after three revisions and is now the only thing left: the PR body still describes
a smaller change than the diff, still says package was "left out of this PR" after fixing
it, still claims a docs/compatibility.md edit that is not present, and still quotes a suite
result that this PR's own TASKS.md explicitly supersedes.

Follow-up on the previous review (ebuild-110-26a767f4.md)

Order note: prior-reviews.txt presents the three earlier reports as "oldest first", but they
are concatenated in short-SHA order. The real order from gh pr view --json commits is
a65206b4 → 64aa812b → c7d83d86 → 26a767f4 → 93fdebdd, so the last review was
26a767f4 — the one printed first. This follow-up is against that one.

Prior finding	Status	Evidence
1 · Med — PR body describes a different change than the diff	untouched	Body at this head unchanged; re-verified below. Carried forward as finding 1.
2 · Med — three mutually exclusive suite results	partially addressed	TASKS.md:52-54 now carries one figure with an explicit "Supersedes any other count quoted … in PR #110's description". The description was not updated, so the superseded number is still the first one a reviewer reads. Folded into finding 1.
3 · Med — T-003/T-004 in Completed with Verified by: self	resolved in 93fdebdd	TASKS.md:14-16: T-002/T-003/T-004 all sit in Active at review; the evidence prose is retained under a heading that names the .ai/reviewer.md boundary verbatim. Exactly the recommended treatment.
4 · Low — test_footprint.py footprint test read the process CWD	resolved in 93fdebdd	monkeypatch.chdir(tmp_path) added at tests/unit/test_footprint.py:239. Caveat below.
5 · Low — gcc_is_missing() imported out of conftest.py	resolved in 93fdebdd	git mv tests/conftest.py tests/support.py (confirmed status: renamed in the API compare), both importers updated, no root conftest.py remains.
6 · Low — helper lives in the Ninja-specific module	accepted, backlog entry not filed	Still ninja_backend.py:44. It was explicitly non-blocking; the recommendation was to file it. No TASKS.md row was added. Restated as finding 3.

Resolved in earlier rounds and re-verified by execution here, so not restated:
a65206b4 #1 (regression test passed against unfixed code — it now fails pre-fix),
c7d83d86 #1 (High — _report_footprint silent on Windows — fixed and covered),
a65206b4 #4 (which is None branch untested — test added),
c7d83d86 #4 (two gcc probes, unreachable inner handler — one probe now, handler dropped).

c7d83d86 #3 (no CI) is still open and is finding 2 below.

Caveat on prior finding 4: the chdir is correct hygiene, but I could not construct a CWD
that changed the outcome. With a root eos.yaml + board.yaml present the test passes
either way, because the assertion is only any("Flash" in line) and format_report emits a
Flash: line whether or not board capacities resolve. Treat it as hardening, not a repair.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	pr.json body vs. ebuild/cli/commands.py:516, :2534, TASKS.md:14-16, :52-54	Fourth review, same finding, body unchanged. Five specific contradictions at this head. (a) "Changes" says the helper is used "so the Ninja edge and ebuild test use the same binary path" and "Use that path in _run_native_tests" — the diff converts four call sites: _run_native_tests (:2624, :2633), package (:2534) and _report_footprint (:516). (b) "Related Issues": "Leaves T-003 (ebuild package unsuffixed .exe path)" — T-003 is fixed here; TASKS.md:15 lists it at review with an evidence paragraph. (c) "Additional Notes": "ebuild package … was left out of this PR so the test-runner fix stays reviewable on its own" — it is in this PR, and the CHANGELOG entry at CHANGELOG.md:15-19 documents it as fixed, so the body contradicts a file in its own diff. (d) "Changes": "Document the fix in README, CHANGELOG, and docs/compatibility.md" — docs/compatibility.md is not among the 10 files; verified git show origin/master:docs/compatibility.md, its Known Limitations table has one row (eOSuite on Windows) and the diff does not touch it. (e) "Testing": "1 failed, 557 passed, 5 skipped, exit code 1", attributing the failure to tests/unit/test_footprint.py::TestMeasure::test_measures_a_real_binary — the very test this diff puts skipif(gcc_is_missing()) on (test_footprint.py:59-62), so at this head it skips rather than fails; and TASKS.md:52-54 declares that count superseded by "560 passed, 6 skipped, exit code 0". T-004 is not mentioned in the body at all. Per brief §5 the unsupported claim is itself the finding. The merge-time risk is concrete: a maintainer reading "Leaves T-003" merges believing package is still broken on Windows.	Rewrite Summary / Changes / Testing / Related Issues / Additional Notes against 93fdebdd: four call sites converted; T-002, T-003 and T-004 all addressed here; one suite result from one run on this head with its host and exit code; drop the docs/compatibility.md claim or add the file. Nothing in the code needs to move.
2	Medium	checks.txt (empty); GitHub API	No CI has run on this head, for the third consecutive review. Queried at 93fdebdd: actions/runs?head_sha=… returns 3 runs — Simulation Test, CI — ebuild, CodeQL — every one status=completed, conclusion=action_required; commits/93fdebdd/check-runs → total_count: 0; commits/93fdebdd/status → {"state":"pending","count":0}. This is a fork PR (headRepositoryOwner: thompsondev) queued behind the outside-contributor Actions approval gate, not a broken workflow — ci.yml triggers on pull_request: branches: [master, main] and the base is master. That matters more here than usually: the entire subject is Windows behaviour, and ci.yml:85 is the only Windows runner in play. mergeStateStatus: BLOCKED with mergeable: MERGEABLE and reviewDecision: REVIEW_REQUIRED.	Maintainer action, not the author's: approve the workflow runs. Nothing the author can push will produce evidence. Same gap ebuild#103 exists to close.
3	Low	ebuild/build/ninja_backend.py:44; TASKS.md:12-16	executable_output_path() is now the cross-module contract for "where the linked binary lands", consumed from ebuild/cli/commands.py at four sites, but it still lives in the Ninja-specific backend. Output-path naming is a property of the toolchain and host, not of Ninja, and commands.py already treats the backend as dispatchable (it imports ninja_command from ebuild.build.dispatch). The previous review rated this non-blocking and asked only that it be put on the backlog; no TASKS.md row was added.	Add a todo row: move the helper to a backend-neutral module (ebuild/build/layout.py) and re-export from ninja_backend for compatibility. Do not churn it in this PR.
4	Low	CHANGELOG.md:9-14; ebuild/build/ninja_backend.py:54-55	Wrong root cause recorded. The CHANGELOG says targets "are linked as <name>.exe on Windows (gcc appends the suffix)", and the helper's docstring says the path matches "the compiler driver's output". Neither is the mechanism here: the link rule is command = $cc $ldflags $in -o $out $libs (:200) and $out already carries the suffix from _exe_suffix() inside _write_ninja (:254) — gcc is handed -o t_smoke.exe and appends nothing. The defect was entirely internal: the generator encoded the naming rule and the four consumers re-derived it without the suffix. As written, the next reader debugging a variant of this looks at compiler behaviour instead of at the duplication. The helper's own first docstring line is already right ("the linked binary path NinjaBackend emits").	Reword to the internal cause: "the Ninja edge already carried the platform suffix (_exe_suffix()); the consumers rebuilt the path without it." Drop "gcc appends the suffix" and "matching the compiler driver's output".
5	Low	TASKS.md:14, :18-54	T-002 — the PR's headline task — moves todo → review with no entry in the new Evidence section, while the two folded-in tasks T-003 and T-004 each get a detailed paragraph. The ledger now documents the derivative work better than the work it derives from, which is the same asymmetry the previous review's finding 3 objected to, inverted.	Add a T-002 bullet naming commands.py:2624/:2633 and tests/unit/test_golden_path_commands.py::TestTestTargetType::test_native_runner_asks_ninja_for_the_linked_binary.

No Critical or High findings. No weakened checks: the skipif added to
test_measures_a_real_binary replaces a probe that aborted collection, gcc is present on
ubuntu-latest so the assertion still runs on the leg that can run it, and the test's inner
try/except FileNotFoundError was dropped in favour of subprocess.run(..., check=True) —
strictly stronger than master. No removed assertions, no widened permissions, no || true.

Verified by execution

Scratch trees only. The local ebuild clone is dirty (TASKS.md,
ebuild/cli/integration.py, tests/ebuild/test_integration_initramfs_security.py staged,
plus untracked smart-sensor/, on branch v90) and was not touched: everything below
came from git archive origin/master (e5d8052) into temp dirs, plus git apply of
diff.patch. Linux, CPython 3.14.4, gcc / ninja / size present, pytest 9.1.1.

git apply --check                                    -> clean against origin/master e5d8052

full suite at head:            566 passed, 0 skipped, exit 0      (pytest -q)

regression value — master + this PR's TEST files only, source fix absent:
  test_native_runner_asks_ninja_for_the_linked_binary   -> FAIL  SystemExit: 1
  test_looks_up_the_windows_suffixed_artifact           -> FAIL  AssertionError: []
  test_it_finds_the_windows_suffixed_artifact           -> module ImportError (helper absent)

head with ONLY the `package` call site reverted to `Path(build_dir) / name`:
  test_it_finds_the_windows_suffixed_artifact -> FAIL
     "[error] No built artifact at _build/node. Run 'ebuild build' first."
  ... 22 passed                                (reproduces the reported symptom exactly)

head with ONLY the OSError guard removed from tests/support.py:
  test_gcc_probe_does_not_raise_when_gcc_cannot_start -> FAIL  FileNotFoundError

remaining unsuffixed executable paths in ebuild/:  none
  (four sites, all via executable_output_path: commands.py:516, 2534, 2624, 2633)

project lint gate, ruff check . --select=E,F,W --ignore=E501:
  master 382 findings / head 382 findings; set difference = one pre-existing
  F541 shifting line 167 -> 188.  Zero new violations.
  (Both are non-zero exit: the gate is already red on master. Pre-existing,
   not this PR — see the run summary.)

So all four regression tests fail against the code they guard, on a POSIX host, which is what
a Windows-only bug's tests have to do. a65206b4's finding 1 — a test that read sys.platform
and therefore passed against the bug — is properly closed: the tests force _exe_suffix()
instead.

Architecture conformance

Conforms. No deviation left.

• §21, Tier 1 – Foundation. Wholly inside ebuild. Nothing added to eos, eBoot or any
  product repo, no new cross-repo dependency, §21.1 not engaged.
• §5.1, architectural law. Direction holds and is unchanged: ebuild/cli → ebuild/build,
  never the reverse. Nothing points up a tier. eBuild stays a build-time control plane, not a
  runtime dependency.
• §9.2, "one source of truth for CLI, VS Code and EoStudio." This is the substance of the
  PR and it is now complete. Before it, the naming rule "linked binary = <name> + platform
  suffix" was written once in _write_ninja and re-derived — wrongly — at four consumer sites.
  The previous review could still say "two of three converted"; at this head all four go
  through one function. That is the rule applied where it actually bites: the writer and the
  readers of build.ninja no longer hold independent copies of the convention.
• §9.2, "actionable diagnostics with remediation guidance." _report_footprint's
  missing-artifact exit now logs (commands.py:518), matching the function's other two early
  returns. Previously the flash/RAM report simply never appeared on Windows with no way to tell
  "skipped" from "not applicable". The single remaining silent return, if not binaries
  (:513), is genuinely not-applicable and needs no diagnostic.
• §9 / §25.1, golden path. build → test → package is intact on Windows for the first
  time; package was the last broken step and it is fixed here.
• §25.2, MLP scope lists memory/flash reporting; that report works on Windows now.
• §28, status and claims policy. Split. TASKS.md follows it well: tasks sit at review,
  the evidence heading names the reviewer boundary, and one suite figure explicitly supersedes
  the others. The PR body does not — finding 1.
• .ai/tooling.md, "exit non-zero on failure, always" — unchanged and still honoured;
  _run_native_tests raises SystemExit(1), package raises on a missing artifact.

The design document is not wrong, stale or silent on anything this PR touches. No proposal
appended.

Proposed changes

Smallest sequence. None of it touches the product code, which I would not change.

1. Rewrite the PR body against 93fdebdd — four call sites, T-002/T-003/T-004 all addressed,
   one suite result from one run with host and exit code, docs/compatibility.md claim
   dropped or the file added (finding 1). This is the only blocker the author owns.
2. Maintainer: approve the queued workflow runs so the Windows leg produces evidence
   (finding 2).
3. TASKS.md: add the T-002 evidence bullet (finding 5) and a todo row for the helper move
   (finding 3).
4. Reword the CHANGELOG parenthetical and the helper docstring to the internal cause
   (finding 4).

Items 3 and 4 are documentation-only. Once item 1 lands and CI is green this is ready.

No fix PR opened: every open finding is Medium or Low (the brief authorises fixes only for
High), and all of them live on fix/windows-ninja-test-target-paths, which belongs to this
open PR and must not be touched.

Not checked

• No Windows verification at all. Everything above is Linux / CPython 3.14.4. The PR's
  whole subject is Windows behaviour. Forcing _exe_suffix() to .exe exercises the
  path-construction logic — which is the entirety of the change — but it is a proxy: no real
  gcc that appends .exe, no real ninja resolving a suffixed edge, no backslash separators,
  no drive letters, no Windows argument quoting.
• The project's own CI has never run on this head (finding 2). Nothing here is confirmed by
  the project pipeline; all execution above is mine, in throwaway trees.
• None of the three suite counts in the PR or TASKS.md was reproduced. My run gives
  566 passed / 0 skipped / exit 0 on a different host and a newer Python (3.14 vs the body's
  3.12); the body's 557/5/1 and TASKS.md's 560/6/0 are both unverified, including the one
  I recommend keeping. My own figure is not a check on theirs — different host, different
  skip set.
• No real ninja build of a suffixed edge and no real efwtool: subprocess.run and
  shutil.which are stubbed in the tests exercised above, so what was verified is the argv
  and the path lookup, not the tool behaviour.
• macOS not exercised. _exe_suffix() returns "" for anything but win32, correct for
  Darwin, but no run confirms it.
• mypy not run (CI installs it at ci.yml:49; not installed here). The performance job
  (ci.yml:90, pytest-benchmark) was not run.
• The >>> block in executable_output_path's docstring is never executed — no
  --doctest-modules in pytest.ini, pyproject.toml or any workflow. Master already carries
  one such block in this module, so this is pre-existing house style, not a finding.
• Whether the branch is rebased on the newest master was not checked directly (the API
  cross-fork compare 404s), but diff.patch applies cleanly to origin/master at e5d8052
  and GitHub reports mergeable: MERGEABLE, so there is no conflict. The author's own
  "Branch is rebased on latest master" checkbox remains unticked.

---

Automated architecture review of 93fdebdd6fcc — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[srpatcha]

Review — ebuild#110 "fix(ebuild): ask ninja for the Windows test binary"

head: 0f8c1b7 author: thompsondev ci: none run (all three workflows action_required; check-runs: 0; combined status pending, count: 0)

Verdict: Follow-up, round five. Round four (0f8c1b71) touched only CHANGELOG,
TASKS.md and one docstring, and it closed the three findings it set out to close plus the
Medium body finding that had survived four reviews. The code is unchanged since
93fdebdd and I re-verified it end to end: four call sites, all through
executable_output_path(), each one individually covered — reverting any one of the four
makes its own regression test fail on Linux. Zero new ruff findings, zero new mypy
findings, 566 passed / exit 0 on CPython 3.12 and 3.14. Nothing the author owns is
open. The only Medium left is not theirs: no CI has run on this PR for the fifth
consecutive review.

Follow-up on the previous review (ebuild-110-93fdebdd.md)

Prior finding	Status	Evidence
1 · Med — PR body describes a smaller change than the diff (five sub-claims)	resolved	Body rewritten (updatedAt: 2026-09-04T14:29:34Z). All five checked at this head: (a) now states "Four call sites, across three consumers"; (b) "Related Issues" no longer says "Leaves T-003" — it lists T-002/T-003/T-004 all at review; (c) the "left out of this PR" sentence is now under "Superseded" and explicitly retracted; (d) the docs/compatibility.md claim is dropped and explained — I confirmed git diff master..HEAD -- docs/compatibility.md is empty, so the file genuinely is not part of this diff; (e) one suite figure only ("560 passed, 6 skipped, exit code 0"), with the superseded counts named as superseded.
2 · Med — no CI has run on this head	untouched	Re-queried at 0f8c1b71: commits/…/check-runs → total_count: 0; commits/…/status → {"state":"pending","count":0}; actions/runs?head_sha=… → Simulation Test, CI — ebuild, CodeQL, all status=completed, conclusion=action_required. Carried forward as finding 1.
3 · Low — helper's module placement not filed on the backlog	resolved in 0f8c1b71	TASKS.md:17 — T-005, "Move executable_output_path() out of the Ninja-specific backend into a backend-neutral module (ebuild/build/layout.py), re-exported from ninja_backend for compatibility", at todo. Exactly the recommended treatment; the helper itself was correctly not churned.
4 · Low — wrong root cause recorded ("gcc appends the suffix")	partially addressed in 0f8c1b71	Both named sites are fixed: CHANGELOG.md:9-16 now reads "The Ninja edge … already carried the platform suffix (_exe_suffix() …), but ebuild test rebuilt the path itself without that suffix", and ninja_backend.py:53-58 replaced "matching the compiler driver's output" with "the same path the Ninja edge in _write_ninja already names via _exe_suffix()". The same wrong mechanism survives in a third place — see finding 2.
5 · Low — T-002 has no evidence bullet while T-003/T-004 do	resolved in 0f8c1b71	TASKS.md:20-26 — a T-002 bullet naming commands.py:2624 and :2633 and the covering test. I verified both line numbers land on the changed lines (grep below); the asymmetry is gone.

Resolved in earlier rounds and re-verified by execution here, so not restated:
c7d83d86 #1 (High — _report_footprint silent on Windows), a65206b4 #1 (regression
test that passed against the bug), a65206b4 #2 (package call site), a65206b4 #4
(which is None branch untested), c7d83d86 #4 (duplicated gcc probe).

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	checks.txt (empty); GitHub API	No CI has run on this head — fifth consecutive review with no project evidence. Three workflow runs exist at 0f8c1b71 and all three are conclusion=action_required: they are queued behind this repo's outside-contributor Actions approval gate (headRepositoryOwner: thompsondev), not misconfigured — ci.yml triggers on pull_request: branches: [master, main] and the base is master. This costs more here than on a typical PR: the entire subject is Windows behaviour and ci.yml:30 (os: [ubuntu-22.04, macos-latest, windows-2022]) is the only Windows runner in play, so the one leg that could confirm the fix natively has never started. mergeStateStatus: BLOCKED, mergeable: MERGEABLE, reviewDecision: REVIEW_REQUIRED.	Maintainer action, not the author's: approve the queued workflow runs. Nothing the author can push produces this evidence. Same gap ebuild#103 exists to close.
2	Low	tests/unit/test_golden_path_commands.py:174	Residual of prior finding 4, in a docstring this PR adds: "On Windows the edge is t_smoke.exe because gcc appends .exe." That is not the mechanism. The link rule is command = $cc $ldflags $in -o $out $libs (ninja_backend.py:204) and $out already carries the suffix via executable_output_path() inside _write_ninja (:257-258), so gcc is handed -o t_smoke.exe and appends nothing. Round four corrected exactly this sentence in CHANGELOG.md and in the helper docstring but not here, so the test that documents the bug still points the next reader at compiler behaviour rather than at the duplication that caused it. (Two other occurrences — test_build_dir_resolution.py:286 and ninja_backend.py:35 — are pre-existing on master at e5d8052 and out of scope.)	Reword to match the CHANGELOG: "On Windows the edge is t_smoke.exe because _exe_suffix() names it that way; a consumer that rebuilds the path without the suffix asks ninja for an unknown target."
3	Low	README.md:104-108	The new paragraph is inserted between a sentence and its own continuation. README.md:100-102 ends "…is a guess presented as a measurement."; :110 begins "It also treats a run that executed no tests as a failure." Both sentences are about ebuild test's result reporting and read as one thought on master; the new type: test/.exe paragraph now sits between them, so "It also" is separated from what it continues. The content is correct and belongs in this section — only the position is wrong.	Move the new paragraph down three paragraphs, after "…trusting the exit status would report a pass", so the reporting discussion stays contiguous.

No Critical and no High findings, and none of the three above blocks merge on the
author's side. Weakened checks, checked specifically: none. The skipif(gcc_is_missing())
added at tests/unit/test_footprint.py:59-62 converts a collection error into a skip —
on master that test called subprocess.run(["gcc", …], check=True) with no guard, so a
Windows host without gcc aborted the file and, with default pytest, the suite. gcc is
present on ubuntu-22.04 and macos-latest, so the assertion still runs on the legs that
can run it. That is an environment guard, not a disabled test. No removed assertions, no
widened permissions, no || true added by this PR.

Verified by execution

Scratch trees only. The local ebuild clone is dirty (TASKS.md,
ebuild/cli/integration.py, tests/ebuild/test_integration_initramfs_security.py staged,
plus untracked smart-sensor/, on branch v90) and the sync step skipped it; it was not
touched. Everything below is a fresh shallow clone of refs/pull/110/head (confirmed
git rev-parse HEAD = 0f8c1b71…, so this is the real head and not diff.patch
re-applied) plus git archive master at e5d8052 for the baseline. Linux, gcc / ninja /
size present, pytest 9.1.1, ruff and mypy from PyPI.

full suite at head:   CPython 3.12.14   566 passed, 0 skipped, exit 0
                      CPython 3.14.4    566 passed, 0 skipped, exit 0

regression value — each of the four call sites reverted individually, in isolation,
with bytecode caching disabled (PYTHONDONTWRITEBYTECODE=1):
  commands.py:2624 -> build_path / t.name          test_native_runner_asks_ninja… FAIL
  commands.py:2633 -> build_path / target.name     test_native_runner_asks_ninja… FAIL
  commands.py:2534 -> Path(build_dir) / …name      test_it_finds_the_windows_suff… FAIL
  commands.py:516  -> build_path / …name           test_looks_up_the_windows_suff… FAIL
                                                   (AssertionError: [] — no report emitted)
tests/support.py with the `except OSError` guard removed:
  test_gcc_probe_does_not_raise_when_gcc_cannot_start -> FAIL FileNotFoundError

remaining unsuffixed executable paths in ebuild/:  none
  four consumer sites, all via executable_output_path:
  commands.py:516, :2534, :2624, :2633  (TASKS.md's cited line numbers are exact)
  nothing else in ebuild/ builds a binary path from a target name — _object_path and
  the lib<name>{.a,.so,.dylib,.dll} rules are separate conventions, correctly not routed
  through this helper

ruff check . --select=E,F,W --ignore=E501:
  master 380 errors / head 380 errors; normalized set difference in both directions is
  EMPTY. Zero new violations. (Both non-zero exit: the gate is already red on master,
  and ci.yml:53 marks it continue-on-error, so it cannot fail the job either way.
  Pre-existing, not this PR.)

mypy . --ignore-missing-imports --no-strict-optional --exclude '^layers/':
  master 13 errors in 7 files / head 13 errors in 7 files; identical finding sets.
  Zero new type errors. (Also continue-on-error in ci.yml:64.)

branch currency: compare master...head -> ahead_by 6, behind_by 0, status "ahead".

So all four regression tests fail against the code they guard, on a POSIX host — which is
what a Windows-only bug's tests have to do — and each one is pinned to its own call site
rather than passing because a sibling site is still correct. That last point is new to this
round: earlier reviews reverted the fix wholesale.

The author's behind_by: 0 also answers the one checklist item they left unticked:
the branch is not behind master. They could not confirm it; it is confirmed.

On the body's "560 passed, 6 skipped" Windows figure — I cannot reproduce a Windows run,
but 560 + 6 = 566, and 566 is exactly what this suite yields on Linux where those six
skips do not apply. That is consistent, not proof.

Architecture conformance

Conforms. No deviation, and nothing changed structurally since the last review.

• §21, Tier 1 – Foundation. Wholly inside ebuild. Nothing added to eos, eBoot or
  any product repo; no new cross-repo dependency; §21.1 not engaged.
• §5.1, architectural law. Direction holds: ebuild/cli → ebuild/build, never the
  reverse. Nothing points up a tier. eBuild remains a build-time control plane, not a
  runtime dependency.
• §9.2, "one source of truth for CLI, VS Code and EoStudio." This is the substance of
  the PR and it is complete and now independently pinned. Before it, "linked binary =
  <name> + platform suffix" was written once in _write_ninja and re-derived — wrongly —
  at four consumer sites. Round four's docstring correction (prior finding 4) matters here
  beyond wording: the helper's docstring now states why it exists ("A consumer that
  rebuilds this path independently … can silently drop the suffix"), which is the rule
  §9.2 is asking for, written where the next consumer will read it.
• §9.2, "actionable diagnostics." _report_footprint's missing-artifact exit logs at
  debug (commands.py:517), matching the function's other two early returns. The one
  remaining silent return, if not binaries (:514), is genuinely not-applicable.
• §9 / §25.1, golden path. build → test → package is intact on Windows, and §9.2's
  per-target flash/RAM report appears there, for the first time.
• §28, status and claims policy. Now satisfied on both sides of the ledger, which is
  the change this round delivers. TASKS.md:19 keeps T-002/T-003/T-004 at review under
  a heading that quotes .ai/reviewer.md's boundary verbatim, and the PR body — which for
  four rounds asserted a smaller change and a superseded count — now matches the diff and
  carries one figure with its host and exit code. §28's "do not represent proposed or
  planned capabilities as implemented" is honoured: T-005 is filed at todo, not claimed.
• .ai/tooling.md, "exit non-zero on failure, always." Unchanged and honoured:
  _run_native_tests raises SystemExit(1), package raises on a missing artifact.

The master design is not wrong, stale or silent on anything this PR touches. No proposal
appended.

Proposed changes

Two one-line documentation edits and one maintainer action. None of it touches product
code, which I would not change.

1. Reword tests/unit/test_golden_path_commands.py:174 to the internal cause (finding 2).
2. Move the new README.md paragraph below the "no tests were found" paragraph (finding 3).
3. Maintainer: approve the queued workflow runs so the windows-2022 leg produces
   evidence (finding 1).

Items 1 and 2 are comment/prose only and cannot affect the suite. With them, or without
them, this PR is done on the author's side — findings 2 and 3 are Low and neither
misleads about behaviour the way the body finding did. What it is waiting on is CI and a
maintainer review, per .ai/reviewer.md's boundary that the implementer cannot sign off
their own work.

No fix PR opened: every open finding is Medium or Low (the brief authorises fixes only for
High), finding 1 is not fixable by a code change at all, and findings 2–3 live on
fix/windows-ninja-test-target-paths, which belongs to this open PR and must not be
touched.

Not checked

• No Windows verification, still. Everything above is Linux. The PR's whole subject is
  Windows behaviour. Forcing _exe_suffix() to .exe exercises the path construction —
  which is the entirety of the change — but it is a proxy: no real gcc, no real ninja
  resolving a suffixed edge, no backslash separators, no drive letters, no Windows argument
  quoting. This gap closes only when finding 1 does.
• The project's own CI has never run on any head of this PR (finding 1). Every result
  above is mine, in throwaway trees, on one OS.
• The PR's "560 passed, 6 skipped" was not reproduced. My 566/0/exit-0 is arithmetically
  consistent with it but is a different host and a different skip set, so it is
  corroboration, not verification. I did not identify which six tests skip on Windows.
• macOS not exercised. _exe_suffix() returns "" for anything but win32, correct for
  Darwin, but no run confirms it.
• No real ninja build of a suffixed edge and no real efwtool: subprocess.run and
  shutil.which are stubbed in the tests exercised above, so what is verified is the argv
  and the path lookup, not the tool behaviour.
• The pytest-benchmark performance job (ci.yml:89) was not run, and neither was
  pytest-cov, so the coverage figure the workflow uploads is unknown here.
• Two repo-level gaps were observed but are pre-existing on master and not attributable
  to this PR, so they are not findings against it: ruff (ci.yml:53) and mypy
  (ci.yml:64) both carry continue-on-error: true, so neither can fail the job, and
  ruff is already 380-red; and NinjaBackend._object_path's >>> block fails as a
  doctest on master and at head, while nothing executes doctests at all (no
  --doctest-modules in pytest.ini, pyproject.toml or any workflow). The new helper's
  own >>> block passes. These belong to the maintenance sweep, not to this author.

---

Automated architecture review of 0f8c1b717cc2 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.