fix(min): close the model file once, not twice, on a short read

[srpatcha]

Problem

stub_load_model() in min/src/runtime.c closes the same FILE * twice when a
model file is opened successfully but read short.

Surfaced by the maintenance sweep's health scan for eAI, section
"C — unused functions and dead code":

min/src/runtime.c:342: error: Resource handle 'fp' freed twice.

Root cause

fp is closed unconditionally as soon as the fread() returns:

size_t nread = fread(buf, 1, (size_t)fsize, fp);
fclose(fp);                       /* line 321 — fp is dead from here on */

if (nread == (size_t)fsize) {
    ...
} else {
    free(buf);
    fclose(fp);                   /* line 342 — closes an already-closed handle */
}

The short-read branch was written as if it still owned the handle. The two
sibling fclose(fp) calls further down (the malloc failure path and the
implausible-size path) are correct: neither is reachable after line 321, because
both belong to else branches of conditions that must have been true for the
fread to have happened at all. Line 342 is the only double close.

Calling fclose() on a stream that has already been closed is undefined
behaviour (C17 §7.21.5.1). In practice glibc dereferences a freed FILE
structure, so the observable outcome ranges from a silent no-op to a
double-free abort in a process that was on its way to a perfectly recoverable
fallback path — stub_load_model() goes on to return EAI_OK in stub mode.

The path is reached whenever fread() returns fewer bytes than ftell()
reported: an I/O error mid-read, a file truncated by another writer between the
fseek/ftell sizing and the read, or a model_path that names something
whose reported size and readable length disagree (/proc entries, a growing
file, some network filesystems).

Fix

Remove the second fclose(fp) and leave a comment recording that ownership of
the handle ends at the earlier close, so the next reader does not re-add it.

Nothing else changes: the branch still frees buf and still falls through to
stub mode, which is the intended recovery.

Files changed

• min/src/runtime.c — one fclose() removed, one comment added.

Expected impact

Removes undefined behaviour from the short-read path of the binary model
loader. No behaviour change on any path that was already well defined: the
successful-load path, the parse-failure fallback and the two other close sites
are untouched.

Risks and compatibility

Low. No API, ABI, wire-format or manifest surface is involved; the edit is
local to a static function. The one way this could be wrong is if some path
reopened fp between lines 321 and 342 — it does not; fp is assigned once, at
the fopen.

Not covered by this change

No regression test is added. The branch fires only when fread() returns short
for a file whose size ftell() already reported, which is not reproducible
portably from a unit test without a filesystem fault-injection layer this repo
does not have. The existing 25-test suite exercises the surrounding loader and
is reported below; it does not cover this branch either before or after the
change. Adding fault injection for the loader is worth doing separately and has
been recorded in the sweep backlog rather than smuggled into this PR.

Verification

Executed in an isolated worktree branched from origin/master:

Check	Result	Duration	Command
build	pass	1s	cmake --build build/host --parallel 4
configure	pass	0s	cmake -B build/host -G Ninja -DEAI_BUILD_TESTS=ON
ctest	pass	0s	ctest --test-dir build/host --output-on-failure --no-tests=error --parallel 4

---

Opened by the scheduled autoreview pipeline (model claude-opus-5), branched from origin/master. No human has reviewed this yet. Close it freely if the fix is wrong - a bad automated PR is a bug worth reporting.

Fixes #43

[srpatcha]

Review — eAI#42 "fix(min): close the model file once, not twice, on a short read"

head: 1b1e20f author: srpatcha ci: pass (but see F1 — the C build/test job never ran)

Verdict: The fix is correct and minimal. min/src/runtime.c:342 was a genuine
double fclose() on the short-read branch; removing it is the right change and nothing
else in the function needed to move. The PR body's claims are backed by real recorded
command output. The one thing worth saying is that CI proved none of it: this repo's
C build-and-test job does not run on master-based pull requests, so the only evidence
for a C change is the author's local run.

Findings

#	Severity	File:line	Finding	Recommended fix
F1	Medium	.github/workflows/ci.yml:5-8	The CI — eAI workflow triggers on pull_request: branches: [main] and push: branches: [main, develop]. The repo's default branch is master (.github/STANDARDS.md — "Every PR merges here"), so test-c, test-python and build-arm never run on any PR or any push. checks.txt for this PR confirms it empirically: only Analyze (python) and assign reported. A C-only change merged with zero C compilation in CI.	Already covered by open PR #39 (ci: run the build-and-test workflow on master). No new PR opened. Land #39 before merging this.
F2	Medium	.github/workflows/ci.yml:63-66	build-arm runs cmake ... 2>/dev/null || true on both configure and build, then unconditionally writes dist/build.txt containing "ARM build complete". The job cannot fail and asserts a success it did not verify; release has needs: build-arm, so the release gate is gated on a check that is structurally green. Per .ai/reviewer.md a || true on a build step is a finding regardless of reason.	Already covered by open PR #41 (ci: make C tests and ARM cross-build fail closed). No new PR opened.
F3	Low	min/src/runtime.c:340-343	Pre-existing, in the exact branch this PR edits: a short read frees buf and falls through to rt->loaded = true; return EAI_OK. The caller is told a model loaded when the file was truncated or the read errored. There is also no ferror(fp) distinction, and after the fix fp is closed at line 321 so the error state is no longer inspectable. Not introduced here and out of scope for a UB fix, but the branch is now permanently "correct-looking", which makes it less likely anyone revisits it.	Separate change: return a distinct status (or at minimum EAI_LOG_WARN the short read as the parse-failure path at line 336 already does) so a truncated model is not silently indistinguishable from stub mode.

No Critical or High findings.

Verification I checked

I read the recorded verification rather than trusting the table in the PR body.
state/verify/eAI__runtime-double-fclose.tsv and its three logs match the body exactly:

• configure — exit 0.
• build — exit 0, 132 objects, min/src/runtime.c.o among them. One unrelated
  warning: framework/src/secure_boot.c:36 #warning announcing the development stub.
• ctest --no-tests=error — 25/25 passed, 0 failed. Real tests, not an empty set;
  --no-tests=error was present, so the 0.02 s total is a genuinely fast suite and not
  a silently-collected-nothing pass.

The body's "Not covered by this change" section states plainly that no regression test
is added and that the existing suite does not cover this branch before or after. That is
accurate and correctly labelled — I am not raising it as an unsupported claim.

I also verified the body's reachability argument independently against the source at
head. fp is assigned once at line 311 and closed at 321 inside if (buf). The two
remaining fclose(fp) calls at 345 and 348 sit in the else arms of if (buf) (line
319) and if (fsize > 0 && ...) (line 317) respectively, neither of which can be entered
after line 321 executes. Both are reachable, both are correct, neither is a double close.
Line 342 was the only one. The argument in the body holds.

Architecture conformance

Conforms. Master design §5.1 — the change is confined to a static function inside eAI
(Tier 3 – Advanced, §21) and adds no dependency in any direction. §16.1 — "eAI is
optional; the kernel cannot require it"; nothing here changes that. No public API, ABI,
manifest field or serialized format is touched, so brief item 8 does not apply. The edit
is one deletion inside a translation unit that is already compiled; no hot path, no
allocation, no new walk (item 9). No duplication introduced (item 10). No documentation
is made wrong by this change (item 11) — the behaviour of every well-defined path is
unchanged.

Proposed changes

None to this diff. It is the smallest correct fix.

Ordering, not content: F1 and F2 are already addressed by open PRs #39 and #41. Merging
those first is what turns this repo's C changes from locally-attested into CI-attested.
Until then every C PR here — including this one — carries exactly the evidence its author
chose to run.

Not checked

• I did not independently re-run the build or the test suite. I read the pipeline's
  recorded logs in state/verify/, which are contemporaneous command output, but I did
  not re-execute them in a fresh worktree. NOT RUN, deliberately — the repo checkout
  is clean and I did not want to create a worktree outside the sanctioned fix path.
• I did not verify the fix under a fault-injection harness, because none exists — the
  short-read branch cannot be reached portably from a unit test. This is the same gap the
  PR body declares. Nobody has executed the corrected branch.
• I did not audit the rest of runtime.c or the other 131 translation units for similar
  double-free / double-close patterns. The Cppcheck run that surfaced this one came from
  the maintenance sweep, not from this review.
• I did not check whether Analyze (python) (CodeQL) covers C at all in this repo; the
  job name suggests it does not, which would mean no static analysis of C runs on PRs
  either. Unverified.

---

Automated architecture review of 1b1e20fb805d — 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.