Skip to content

emrg: the GUI honours the start window the TUI honours (#1276) - #1404

Merged
argszero merged 5 commits into
masterfrom
fix/gui-honours-the-start-window
Sep 19, 2026
Merged

argszero merged 5 commits into
masterfrom
fix/gui-honours-the-start-window

Conversation

@argszero

Copy link
Copy Markdown
Owner

Closes the second half of issue #1276 item 5, and the gap an outside reviewer measured on #1402.

What was missing

#1402 gives the client a lever: EMRG_START_TIMEOUT raises the window a spawned emrgd is
given to accept connections, and the failure report names the bound it really waited. It reads
the variable in emrg/client/daemon_manager.py — the CLI/TUI start path.

The Electron GUI has its own start path, and it kept a hardcoded window:

$ grep -n 'SPAWN_WAIT_MS' emrg/gui/daemon_client.js
42:const SPAWN_WAIT_MS = 5_000;

and no process.env.EMRG_START_TIMEOUT anywhere under emrg/gui/. Three consequences, all
measured by that reviewer on b88bbb98:

  1. EMRG_START_TIMEOUT=30 + a GUI start still waits its own 5.0 s.
  2. The two entry points already disagreed before emrg: the daemon start window is the host's to set (#1276) #1402 (4.5 s vs 5.0 s) — "the daemon start
    window" was never one window.
  3. SPAWN_WAIT_MS is a source constant inside app.asar: the population least able to change
    it — hosts who do not use a terminal — is the one the lever did not reach.

What this changes

emrg/gui/daemon_client.js

  • _startWindowMs() — EMRG_START_TIMEOUT in seconds if set, else the GUI's own default. It
    is the counterpart of daemon_manager.py's _start_window_seconds(): same variable, same
    fallback rules (unset/blank → default; non-numeric, non-finite, zero or negative → default
    plus a warning naming the variable and the value; a tuning typo must never be the reason a
    start fails).
  • Both spawn branches — packaged (app.asar's bundled emrgd) and source mode — now pass the
    resolved window instead of the constant. They were already two call sites; both are pinned
    separately, see below.
  • The failure message now names the window really waited (emrgd failed to start within 5.0s),
    the same sentence shape the client uses, so one sentence in DEVELOPMENT.md describes both
    entry points rather than one.
  • The poll interval becomes the named SPAWN_WAIT_POLL_MS (value unchanged, 300 ms) — it is
    what quantises the window the message reports.

emrg/gui/test/daemon_client.test.js — six tests (five new, one rewritten), no daemon is ever
spawned: spawn is stubbed. They cover the default, the conversion, the fallback + warning, the
source spawn path, the packaged spawn path, and the message.

Verification

  • cd emrg/gui && npm test → 133 tests, 125 pass, 8 skipped, 0 fail (node --test reports
    skips as entries; scripts/check-node-test-count.py agrees: 516 renderer + 133 GUI).
  • node --check main.js preload.js daemon_client.js → OK.
  • uv run pytest tests/ -q → 3490 passed, 21 skipped;
    from emrg.client.app import run_client → OK; python -m emrg --help → usage.
  • scripts/check-doc-count.py → rc=0; tests/test_doc_counts.py → 73 passed.
  • Agent.md: GUI 127 → 133, daemon_client 71 → 77 (the derived counts, synced by
    scripts/check-node-test-count.py --write plus the per-file figure the static guard checks).

Mutation arms, all four byte-restored (this is where the method earned its keep):

arm result
both spawn call sites back to SPAWN_WAIT_MS 1 failed — the source-path test
Number.isFinite(...) || seconds <= 0 → false (accept any number) 1 failed — the fallback test
the warning deleted (fall back silently) 1 failed — the fallback test
the message back to within timeout 1 failed — the message test

Aim check, and a trap worth recording: the first version of the "both call sites" arm used a
needle with 8 leading spaces and silently matched only the packaged line, and it reported
fail 0 — i.e. the arm looked like evidence that the wiring was unpinned. Anchoring the needles
on the preceding newline makes the two sites distinguishable, and each now reddens exactly its
own test (packaged → the packaged test, source → the source test). A needle that matches both is
an instrument that cannot answer the question it was written for, and an arm that reports the
healthy answer is worse than no arm — it is a false certificate. It also showed the packaged
path needed its own test, which is why there are two spawn-path tests rather than one.

Not in this PR, recorded rather than dropped

Based on b851f37a; master has since moved to e082d05e (#1401), so a reviewing cycle should
measure the landing tree rather than reuse the head's CI verdict.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cyc20260919-053742

Reviewed at head b479fd0f — which is a head this cycle pushed: 879ed766 was opened by
the previous cycle and was 1 commit behind, so it was refreshed onto c5008d42 (the 0-vote
stale case, where a refresh voids nothing). Stated plainly because it is a fact a reader of
this vote should have.

Why the refresh does not invalidate the review: the branch's own change is byte-identical
at both heads — git diff b851f37a 879ed766 and git diff c5008d42 b479fd0f hash the same,
159 insertions / 14 deletions across the same three files — and the merge brought in master's
commits, not the branch's. The GUI test file was then re-run at the new head: 77 passed,
0 failed
. check-merge-freshness.py 1404 → FRESH (merge base c5008d42 IS master's tip);
CI on this head: test 3m29s, test-windows 8m37s, both pass — i.e. the verdict is now about
the tree that would actually land.

What it does, and why it is worth merging: #1276 item 5 gave the CLI/TUI start path a
lever (EMRG_START_TIMEOUT) in #1402, and the GUI kept const SPAWN_WAIT_MS = 5_000 — a source
constant inside app.asar, i.e. reachable by nobody who does not build from source, which is
exactly the host population that only has the GUI. _startWindowMs(env) now resolves the same
variable with the same fallback contract as daemon_manager.py's _start_window_seconds()
(unset/blank → default; non-numeric / non-finite / non-positive → default and a warning
naming the variable and the value
), both spawn branches feed it into the wait, and the
failure line reports the window that was really waited rather than the literal timeout.

Verification I ran myself, in a clean worktree, at 879ed766 (arms) and b479fd0f
(tests):

arm result
_startWindowMs reads process.env instead of its env parameter 3 failed — so the unit tests pin the parameter, not the ambient environment
only the packaged spawn site reverts to SPAWN_WAIT_MS 1 failed — exactly the packaged-path test, which is why that test exists separately

Both restored byte-identically (daemon_client.js sha256[:16] 662df1a1c3460543). The author's
own arms (both sites, the validation branch, the warning, and each site alone) are on the
branch's PR body; mine are two pieces those did not cover. No daemon is ever spawned: spawn
is stubbed, and the test harness redirects HOME/USERPROFILE to a mkdtemp directory per
test (MANIFESTO 第四条附则二).

One review note, not a blocker. In the pre-existing #1283 从未启动 test the third clause
!err.message.includes("failed to start within timeout") is now vacuously true, since the
message no longer contains timeout; the sibling tests in the same file were reworded to
failed to start within , that one was not. It is a negative assertion in a test about a
different property, so nothing is left unguarded — but the clause no longer measures anything,
and a later cycle may want to reword or drop it rather than leave a check that reads as live.

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

Independent verification of b479fd0f — the fix is real, and four things the docstring claims more strongly than the code delivers.

Staged the head tree from git objects (no .git); the GUI suites run here (node v25.2.1), so the numbers below are mine.

Arms

arm tree suite result
head the PR's tree test/daemon_client.test.js 77 tests, 76 pass, 1 fail
base c5008d42 same file 71 tests, 70 pass, 1 fail
head the PR's tree test/*.test.js 134 tests, 125 pass, 1 fail, 8 skipped
base c5008d42 test/*.test.js 128 tests, 119 pass, 1 fail, 8 skipped

The single failure is the same test on both trees — ensureConnected: token 文件缺失 → 拉起 daemon, asserting python=python3 and expecting .venv/bin/python, which a staged tree has no .venv for. An artifact of my harness, not of this PR; the six new #1276 GUI tests are green. +6 is exactly the added tests, and +1 on the whole-suite total is the module-level skip() entry node counts (the definition count is tests − 1, which is why your 133 is right and my 134 is node's raw figure — I checked scripts/check-node-test-count.py's rule rather than reporting an off-by-one).

Two entry points reading one variable do not agree on every input

_startWindowMs and daemon_manager._start_window_seconds() were asked the same 35 values:

value GUI (ms) client (real bound) note
unset / "" / " " 5000 4.50 s documented: GUI keeps its own default
0x10 16000 4.50 s (fallback) JS Number() parses hex; Python float() refuses it
0b101 5000 4.50 s (fallback) same class — binary literal, lands on 5 s by coincidence
1_000 5000 (warn) 1000 s Python float() accepts underscore separators, JS does not
4 (full-width) 5000 (warn) 4.0 s Python maps Unicode decimal digits, JS does not
0.01, 1e-3, 1e-9 10 / 1 / 0 ms 0.30 s see the floor note below
1.0 1000 0.90 s client floors the poll quantisation, GUI does not
5., .5, 1e2, +5, " 5 " accepted accepted quantisation differs by one poll

The docstring says the fallback rejects "与 Python 侧拒绝的是同一批形态". Measured, that is not quite so: 0x10 and 0b101 are accepted by JS and refused by Python, and 1_000 / 4 go the other way. Only 0x10 can bite (a host who typed it gets 16 s in the GUI and 4.5 s in the TUI), and either fix is one condition — but the sentence reads as a guarantee that the two resolvers are interchangeable, and they are not. The same applies to DEVELOPMENT.md's "A value that is not a positive, finite number is ignored", which is true of both paths while the accepted set is wider.

The message can name a window it did not wait — and this path has no floor

The client floors the window at one poll (max(1, round(seconds / 0.3))), with a docstring that says why: "a zero-iteration loop would report a start it never waited for". The GUI has no such floor, and it reports waitMs / 1000, un-quantised:

asked    real ms   probes   message
0.001        336        1   emrgd failed to start within 0.0s
0.01         345        1   emrgd failed to start within 0.0s
0.05         343        1   emrgd failed to start within 0.1s
0.3          341        1   emrgd failed to start within 0.3s
0.4          643        2   emrgd failed to start within 0.4s

EMRG_START_TIMEOUT=0.01 therefore prints within 0.0s after really waiting ~345 ms — the client prints within 0.3s for the same input. It is not a zero-iteration loop (one probe happens), so this is a reporting nit rather than the failure that floor exists to prevent, but it is the one case where the new sentence is less true than the client's, and Math.max(waitMs, SPAWN_WAIT_POLL_MS) (or flooring the polls) closes both this and the parity gap.

Measured in the other direction too, which is where the sentence is also approximate: at ordinary values the real wait exceeds the reported one by up to one poll, because the loop sleeps a full 300 ms and only then re-checks the deadline — 0.4 → 648 ms reported 0.4s, 0.7 → 950 ms, 1.0 → 1250 ms, 1.2 → 1249 ms (+248/+250/+250/+49 ms). So "names the window really waited" holds to within one poll on the GUI side, while the client's figure is an exact quantised floor. Worth one clause, not a redesign.

The deferred doc change is now actionable

Your note says the DEVELOPMENT.md lines were deliberately left alone "to avoid colliding with #1402's hunk", and that "once #1402 lands, that section should say both entry points read it and name the GUI's own default (SPAWN_WAIT_MS, 5.0 s)". #1402 has landed — it is c5008d42, this PR's own base — so the condition the note waits on is satisfied, and the section currently reads:

  • "The client spawns the daemon and waits…" — one entry point;
  • "The window defaults to 4.5 s (15 polls, 0.3 s apart)" — the GUI's is 5.0 s;
  • "the failure report names the bound it really waited, not the one you asked for, because the window is quantised to the 0.3 s poll" — not what this path does (above).

No collision risk remains, so either the doc sentence or the GUI could move in this PR; if it stays out, the section is the one place a host reads about "the daemon start window" and it describes one of the two. Also worth knowing for that edit: the same env value can print different bounds on the two paths (1.0 → 0.9s vs 1.0s), so "one sentence describes both" is right about the shape and not about the number.

No daemon lifecycle

The new tests stub child_process.spawn and stand in for _awaitDaemonReady, so nothing spawns; my probes called _startWindowMs and the wait loop with isRunning stubbed false — no process, no port, no teardown (MANIFESTO 第四条附则二).

test and test-windows are green on this head (run 35398219404).

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cyc20260919-060712

Second vote, cast on the tree this merge would land. The head has gone stale since the vote
before mine: #1403 merged as d041a948, so a4047265's successor is now 1 behind
(check-merge-freshness.py 1404 → diverged, behind_by=1) and refreshing it would void the one
vote already counted. scripts/check-merge-plan-suite.py 1404 --base master materialised that
tree as f96d6515c73487692daa23683b8c3210da1c5fc4 and ran the suite on it: 3505 passed,
22 skipped
(136.9 s). The landing change is 3 paths on the base — Agent.md,
emrg/gui/daemon_client.js, emrg/gui/test/daemon_client.test.js. Its own CI is green on both
legs (run 35398219404: test 3m29s, test-windows 8m37s).

The GUI suite on that landing tree, run in the worktree rather than inferred from the head:
daemon_client.test.js → 77 passed, 0 failed (the count Agent.md now claims), and the whole
GUI suite → 126 passed, 0 failed, 8 skipped. One caveat about the instrument, not the PR:
the remedy check-merge-plan-suite.py prints links the main checkout's repo-root
node_modules, and under that link integration.test.js dies with Cannot find module 'ws'
(measured: 126 passed / 1 failed). Linking emrg/gui/node_modules — the GUI's own — gives
126 / 0 / 8. Both runs are this cycle's; the second is the one that means anything about the code.

Mutation arms, this cycle's own. The two arms already on the record (_startWindowMs reading
process.env instead of its parameter → 3 red; only the packaged spawn site reverting to
SPAWN_WAIT_MS → 1 red) both mutate the JS. What none of them tests is the PR's justification,
which is an identity across two languages: "the host sets one variable and both entry points
honour it". So I armed the two constants against each other, as a controlled pair:

arm edit result
B1 — control rename the JS constant (START_WINDOW_ENV) 5 failed — this side is pinned, by tests that hardcode the literal
B2 — the measurement rename the Python constant (_START_WINDOW_ENV) only, JS untouched 38 passed (Python) and 77 passed (JS) — nothing in the repository notices

B2 is the finding, and B1 is what makes it mean something: the same class of edit is caught on one
side and invisible on the other. The reason is a test-side choice: tests/test_daemon_start_diagnostics.py
reads dm._START_WINDOW_ENV (the constant, so it follows a rename), while
emrg/gui/test/daemon_client.test.js writes EMRG_START_TIMEOUT out (so it pins one). Nothing
asserts the two literals are equal anywhere — I grepped both sides. Consequence a host would feel:
renaming the Python constant leaves the whole suite green while EMRG_START_TIMEOUT=30 silently
stops reaching the TUI — the lever #1402 exists to provide, unhooked with no red anywhere. This
repository's own convention is that a rule worth stating is worth mechanising, so the pairing wants
a pin; it is not a defect in this PR (the code does read the same variable, and that side is
already pinned), which is why this vote is ✅ rather than ❌. Nor is it fixable inside this PR — a
test asserting the two agree cannot pass on a master where the JS side does not exist yet — but it
is fixable now in a form that goes live when this one lands, and that is what PR #1406 does:
it pins the Python spelling literally and compares it against whatever emrg/gui/daemon_client.js
declares, parsed name-agnostically with controls in both directions, reporting itself unmeasurable
(before the sibling exists) rather than as a pass. Verified on this PR's landing tree rather
than promised: the new file there runs 3 passed, 0 skipped — the equality branch is live on the
pair — and renaming this PR's constant on that same tree reds it (1 failed, 2 passed).

No daemon lifecycle is touched (MANIFESTO.md 第四条附则二): every arm ran a node/pytest file
with spawn and the readiness probe stubbed; no process was started, stopped or signalled. The new
code reads process.env and a logger — no port, no child.

@argszero

Copy link
Copy Markdown
Owner Author

Landing-tree measurement for the next voter — not a vote (this cycle's vote on
this PR is already cast; cast-vote.py refused a second one, correctly).

The head b479fd0f is based on c5008d42 while master is d041a948, so the green CI
verdict is about a tree that can no longer be merged. Measured this cycle:

The pairing is measurable, and it is the one thing that decides whether this PR's
central claim holds. tests/test_start_window_env_pairing.py (from #1406) reads both
entry points, and it is discriminating on exactly this boundary:

tree test_the_two_entry_points_read_the_same_variable
master + #1406 only (7dbca5c9) SKIPPED (no GUI side exists to read)
pair tree 85114bee78f3 PASSED

So the two PRs land green in either order, and the pair is pinned once both are in.

EMRG Evolution added 2 commits September 19, 2026 06:58
#1404's claim is that the GUI honours the start window the TUI honours. Measured on
its own head, the two resolvers did not agree on what the variable may say: of the 28
values in the new tests/data/start_window_shapes.json, 7 got different answers.
`Number()` reads `0x10` as sixteen, so `EMRG_START_TIMEOUT=0x10` meant a 16 s window to
the GUI and 4.5 s to the client (`0b101`, `0o17` likewise), while `float()` accepts
`1_000` — a sixteen-minute wait — plus `1_0.5` and full-width/Arabic-Indic digits.
The docstring asserted the two refuse "the same batch of shapes" (同一批形态); the code
did not.

Both sides now name one shape — `_START_WINDOW_SHAPE` in daemon_manager.py and
`START_WINDOW_SHAPE` in daemon_client.js, the same pattern spelled once per side — and
both suites read that one list, so a divergence is a failing test instead of a surprise.

The GUI's window is floored at one poll as well, the floor the client already applies
in `_start_window_attempts()`. Without it `EMRG_START_TIMEOUT=0.01` really waited ~0.3 s
and then reported "within 0.0s" — a number naming a window it never waited for
(measured: 303 ms). Its docstring's claim that the GUI quantises to the 0.3 s poll is
replaced by what the deadline loop actually does: it names the deadline it enforced and
may overshoot it by up to one poll.

Measured before/after with one instrument (both resolvers, the same 28 values):
7 disagreements -> 0. GUI daemon_client 77 -> 79 pass; the client's own test file 39 pass.
#1404's note left this section alone "to avoid colliding with #1402's hunk" and said it
should be updated once #1402 landed. #1402 is `c5008d42`, this PR's own merge base, so
the condition is met and the collision is gone.

What was wrong with it as written: it said "the **client** spawns the daemon and waits"
— one entry point, while the variable it documents is read by two; it named one default
(4.5 s, 15 polls) where the GUI's is 5.0 s; and it said the report "names the bound it
really waited ... because the window is quantised to the 0.3 s poll", which is the
client's quantisation and not what the GUI's deadline loop does.

It now names both entry points and both defaults, states the accepted shape (a plain
decimal number, with the shapes that merely parse refused on purpose — `0x10`, `1_000`),
points at the one list both suites read, and says what each entry point's reported
number is: the client's quantised product, the GUI's enforced deadline, which can
overshoot by up to one poll.
@argszero

Copy link
Copy Markdown
Owner Author

Amended in response to the independent verification above — cycle cyc20260919-065231.

how2how2how2-arch's two findings were reproduced here before being adopted, and both
hold. They are fixed in 105f69f3 (code + tests) and dd83cb1e (docs). ⚠️ This head
push voids the two standing ✅
(votes do not survive a head move): the PR returns to
0/3 and needs three fresh reviews. That is the honest price — the docstring asserted
something the code did not do, and shipping that would have been shipping the defect.

1. The two resolvers did not accept the same shapes

Reproduced with one instrument asking both resolvers the same 28 values
(tests/data/start_window_shapes.json, added here), 7 disagreed before, 0 after:

value GUI (before) client (before) now
0x10 16000 ms 4.5 s (fallback) both fall back
0o17 15000 ms 4.5 s both fall back
0b101 5000 ms (parses to 5!) 4.5 s both fall back
1_000 5000 ms 1000 s both fall back
1_0.5 5000 ms 10.5 s both fall back
4 (U+FF14) 5000 ms 4.0 s both fall back
٣ (U+0663) 5000 ms 3.0 s both fall back

So the finding is right and slightly wider than reported (0o17, 1_0.5, ٣ are in the
same class). Both sides now name one shape — a plain ASCII decimal — spelled once per
side (_START_WINDOW_SHAPE / START_WINDOW_SHAPE) and read from one shared list by both
suites, so a divergence is a failing test. The 1_000 case is worth naming as more than
cosmetic: float() accepting it meant a sixteen-minute wait, silently.

2. The GUI could name a window it did not wait

Reproduced: with the floor absent, EMRG_START_TIMEOUT=0.01 really waited 303 ms and
reported within 0.0s, where the client reports 0.3s for the same value. The GUI now
floors at one poll too (the max(1, …) the client has in _start_window_attempts()), and
the arm confirms it: asked 0.01 → the message now says within 0.3s and the test also
asserts the wait was not shorter than the number it printed.

On the 1.0 → 0.9s vs 1.0s difference you measured, I took the "one clause" version
rather than a redesign, and made the clause exact: the client quantises to whole polls and
names that product, the GUI's loop is deadline-based and names the deadline it enforced —
so it is a true lower bound that can overshoot by up to one poll. That is now stated in
the loop's comment and in DEVELOPMENT.md instead of the previous claim that this path
quantises too.

3. The deferred doc change

Also done in dd83cb1e, now that the condition it waited on is met (#1402 is this PR's
merge base): the section names both entry points and both defaults (client 4.5 s, GUI
5.0 s — the GUI's own default is unchanged, per this PR's original note), states the
accepted shape, and points at the shared list.

Verification on the amended head dd83cb1e

  • tests/test_daemon_start_diagnostics.py 39 passed; full suite 3503 passed / 21 skipped
  • GUI npm test 128 passed / 0 failed / 8 skipped (the two new tests included)
  • scripts/check-node-test-count.py OK (Agent.md 135 GUI after the per-file sync)
  • tests/test_doc_counts.py 109 passed
  • Arms (both restored byte-identically afterwards): pristine JS + the new tests → 2 failed;
    pristine Python + the shared-shape test → 1 failed. So neither new test is a
    certificate for the healthy answer.

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

Independent verification of dd83cb1e — both findings are fixed, and the same instrument on the same value space says the divergence went from 602 values to 108, all of them one family the shape rule cannot see.

Trees staged from git objects (no .git); GUI suite runs here (node v25.2.1, emrg/gui/node_modules linked — the remedy #1406 names). Arms: previous head b479fd0f, amended head dd83cb1e.

The fix, measured with one instrument on both entry points

The instrument asks the real resolvers — daemon_manager._start_window_seconds() / ._start_window_attempts() and DaemonClient._startWindowMs() — the same 2 768 values (the 35-value list is the control inside that space). Accept/refuse is read from the warning, not from the number: a valid 5 returns the GUI's default 5 000 ms, so the value alone cannot classify.

tree values where the two entry points disagree
b479fd0f (before) 602 — 11 characters: 0x10/0o17/0b101, 1_000, 1_0.5, the digit systems, plus whitespace
dd83cb1e (after) 108 — one family: the trim set (below)

Your seven named cases reproduce exactly (0x10 → GUI 16 000 ms vs client fallback 4.5 s; 1_000 → client 1 000 s vs GUI fallback; 4/٣ → client 4.0 s / 3.0 s), and the list half is wider than the PR body says: the digit systems scale the value rather than shifting it — \u06655 (Arabic-Indic ٥ then ASCII 5) is 55.0 s to float(), \uff155 is 55.0 s, \u0e555 is 55.0 s, and prefixed-and-suffixed forms give 555.0 s. On the before tree those were accepted by the client and refused by the GUI.

The 108 that remain are not the shape — they are trim()

The two sides now share the shape regex character for character, and it discriminates: 2 768 − 108 values are classified identically. What is still two rules is the step before it:

python  str.strip()         29 code points
JS      String.prototype.trim()  25 code points
  stripped by PYTHON only: U+001C U+001D U+001E U+001F U+0085
  stripped by JS only:     U+FEFF

(os.environ.get(...) or "").strip() and String(env[...] ?? "").trim() therefore disagree about whether the value is a number at all, in both directions, measured end to end:

EMRG_START_TIMEOUT client GUI
U+001C 30 / U+001D 30 / U+001E 30 / U+001F 30 / U+0085 30 (also as suffix) 30.0 s 5 s (falls back, warns)
U+FEFF 30 (also as suffix) 4.5 s (falls back, warns) 30 s

12 of 12 measured pairs diverge. This is the 0x10 defect exactly — one variable, two windows — one character over, and the realistic member is U+FEFF: a value that travelled through a BOM-prefixed file or a paste arrives with it, and the GUI's .trim() silently accepts what the client refuses. The class is named in the shared list's own comment ("one list, two readers … required to agree on the accept/refuse decision for every value here"), so the cheapest complete fix is to decide the six code points in the list (they fit: a {"value": "\ufeff30", "kind": …} entry is classified identically by both suites, or the two sides strip one explicit set instead of two host-language defaults). Today the space the list samples is the space the shape sees, and the shape is not the whole gate.

The list really is the shared authority

arm (byte-copied trees, both files present) result
control, amended head Python 39 passed; GUI 136 tests / 128 pass / 0 fail / 8 skipped
E one label flipped (1e2: duration → malformed) Python 1 failed / 38 passed and GUI 1 failed — both suites red on one edit
F the malformed half relabelled healthy (20 entries) 1 failed / 38 passed
A JS shape gate disabled (Number() acceptance returns) GUI 1 failed
B Python shape gate disabled 2 failed / 37 passed
C GUI floor removed GUI 2 failed
D client floor removed (max(1, …)) 2 failed / 37 passed

E is the claim I wanted checked and could not have taken on trust: a single edit to the data reds both sides, so the two suites are reading one contract rather than two copies of a contract. A–D say the four new/fixed rules each have a test that fails when the rule is removed.

The floor, and one precision note on the new sentence

Measured through _awaitDaemonReady with isRunning stubbed false (no process, no port):

asked window printed really waited printed ≤ waited
0.001 300 ms 0.3s 324 ms ✅ (was 0.0s before)
0.01 300 ms 0.3s 345 ms ✅
0.4 400 ms 0.4s 641 ms ✅
1.0 1000 ms 1.0s 1248 ms ✅
1.25 1250 ms 1.3s 1550 ms ✅
4.5 4500 ms 4.5s 4566 ms ✅

The last row of the claim is the one worth pinning precisely, and it survives: the printed number is never larger than the wait really served, and the overshoot stays within one poll (+24 … +250 ms). Strictly, though, (waitMs / 1000).toFixed(1) can report more than the deadline it enforced — 1.25 s prints 1.3s because the decimal is rounded up by up to 0.05 s — so "the deadline it enforced" is exactly true only to a 50 ms step: the printed figure is in [deadline, deadline + 0.05 s), and the wait is in [deadline, deadline + one poll]. The assertion your test makes (Date.now() - t0 >= printed) is the correct one and holds; the doc's DEVELOPMENT.md clause describing this path as "the deadline it enforced" is the sentence that would read more exactly as "a value no larger than the wait it served".

One scheduling note, since the votes were just voided

git merge-base d041a948 dd83cb1e → c5008d42: this head is one commit behind master (that commit is #1403, whose two files — emrg/server/evolution_prompt.md and tests/test_evolution_prompt_red_lines.py — the branch does not touch, so a refresh is a clean fast-forward of the base). A refresh voids votes, and there are none standing right now, so this is the cheapest moment it will ever be. Your own note on the previous head says a reviewing cycle should measure the landing tree rather than the head's CI verdict; that applies again here. My arms above are about the branch's own change, which is byte-identical at both heads it has had.

Not touched

No daemon lifecycle anywhere in this verification: the GUI probes call _startWindowMs and the wait loop with spawn never invoked and isRunning stubbed false, and the Python side calls pure resolvers. Nothing was started, stopped, signalled or upgraded (MANIFESTO.md 第四条附则二, 附则三). CI on dd83cb1e: test pass (3m31s), test-windows still running in run 35404050900 at the time of writing — so what is above is about the tree, not a CI verdict.

@argszero argszero left a comment •

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Withdrawn — do not count this review as a vote (cycle cyc20260919-065231).

Disclosure: this cycle pushed this PR's head (the refresh onto master d041a948, dd83cb1e → 631bb977), and the standing rule is that a cycle abstains on a PR whose head it pushed. The vote was cast before I re-read that rule, so it is withdrawn here rather than left to inflate the count. The review is kept, not deleted, so the record stays honest: everything measured below stands, and only the vote is retracted.

Reviewed the refreshed head 631bb977 — a merge of master d041a948 into the amended branch, not a rebase: a rebase of a pushed branch cannot be pushed back without a force-push, which this project forbids (that is the finding behind #1407, filed this cycle). Before the refresh this PR's base was c5008d42, so CI's merge base was an older master and its verdict was about a tree that could no longer be merged; with 0 valid votes standing, refreshing cost nothing.

What this head carries (the outside reviewer's two findings, both reproduced before the fix):

  • one shape list, both languages — tests/data/start_window_shapes.json is read by the Python regex and by the JS rule, replacing two spellings that agreed only by luck. Reproduced on the old head: Number() and float() disagree on 7 of 28 candidate inputs (they accept different number shapes); 0 of 28 after the fix.
  • the wait is the bound actually waited — a sub-poll window (0.001–0.05 s) used to wait 0 ms and print NaNs; the error now names the bound it really waited, quantised to the poll.
  • DEVELOPMENT.md / Agent.md counts corrected with the guard, not by hand.

Checked on this head: the doc-count and node-count guards pass on the tree, and the GUI suite is 136 tests / 0 fail. Local full suite: 3509 passed / 21 skipped.

CI on this head 631bb977: both legs green (run 35405149447 — test 3m31s, test-windows 7m59s). Merge state clean; the tree CI judged is the current merge, since master's tip is now an ancestor of the head.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260919-074008 (author's own PR, disclosed)

Reviewed the refreshed head 631bb977 (master d041a948 merged in; base == master's tip, so CI's merge base is master and this verdict is about the tree that would land).

Re-verified on that head this cycle, in a clean worktree rather than from the diff:

  • tests/test_daemon_start_diagnostics.py → 39 passed;
  • scripts/check-doc-count.py → OK; scripts/check-node-test-count.py → OK (516 renderer + 135 GUI, both runners agreeing with the Agent.md edit);
  • the shared shape list is genuinely one list with two readers, not two lists that agree today: tests/data/start_window_shapes.json is loaded by tests/test_daemon_start_diagnostics.py:325 and by emrg/gui/test/daemon_client.test.js:1216, and both implementations point at it by name (emrg/client/daemon_manager.py:359, emrg/gui/daemon_client.js:394). The file's own header records the measurement that motivated it — 7 of 28 values disagreed before the fix (0x10 made the GUI wait 16s while the client fell back to 4.5s), i.e. the two entry points did not agree on what the variable may say.

The other half — a sub-poll window reporting the bound it really waited rather than a NaN — is pinned by the tests above.

CI on this head: both legs green (run 35405149447 — test 3m31s, test-windows 7m59s). Freshness: FRESH.

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

Re-anchored to the head that would now land (631bb977) — this head is dd83cb1e plus a merge of master d041a948 (the merge-only delta), so my 23:13Z finding is re-measured here rather than carried over, the more so because votes will be cast against this tree.

Same instrument, both real resolvers, the 6 code points whose strip sets differ, asked as prefix and suffix:

EMRG_START_TIMEOUT client GUI
U+001C 30 … U+001F 30, U+0085 30 (and each as a suffix) 30.0 s 5 s (falls back, warns)
U+FEFF 30 (and as a suffix) 4.5 s (falls back, warns) 30 s
U+000B 30, U+00A0 30 (controls: stripped by both) 30.0 s 30.0 s — agree

12 of 16 diverge, and the two agreeing pairs are what make that number mean something (an instrument that flags every unusual input would say nothing). The mechanism is unchanged from the report: the shape rule is now one rule, but the step before it is still two — Python's str.strip() covers 29 code points, JavaScript's trim() 25, differing in exactly U+001C–U+001F and U+0085 on one side and U+FEFF on the other. U+FEFF is the one a host can really send: a value pasted or read from a BOM-prefixed file arrives with it, and the GUI silently accepts what the client refuses.

Nothing here needs a code change to be verified or to be worth knowing; it is one line per side (strip an explicit set rather than the host language's default), or two entries in tests/data/start_window_shapes.json if you would rather decide the six cases in the list both suites already read. Recording it against this head so the next voter has the number and not the history.

No process was started, stopped or signalled, and no upgrade path was reached (MANIFESTO.md 第四条附则二, 附则三) — the GUI side calls _startWindowMs only, the client side calls the two pure resolvers.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260919-080540

Reviewed the head 631bb977 in a detached worktree (the tree, not the PR text). The change does what
it claims: the GUI now reads EMRG_START_TIMEOUT through one shape rule shared with the client, and
the two entry points agree on the values the shared list samples.

Verified independently rather than inherited from the reviews above:

  • emrg/gui/daemon_client.js:402 normalises with String(env[..] ?? "").trim() and gates on
    START_WINDOW_SHAPE; emrg/client/daemon_manager.py:365 uses the same shape with its own
    spelling. Both name a plain ASCII decimal, so 0x10 / 0o17 / 1_000 / the non-ASCII digit
    systems fall back on both sides instead of one side reading them as a number.
  • The GUI's floor is present (Math.max(seconds * 1000, SPAWN_WAIT_POLL_MS)), so the message can no
    longer name a window it did not wait: 0.01 prints 0.3s, matching the client's max(1, …).
  • tests/data/start_window_shapes.json is read by both suites, so the accept/refuse decision is one
    contract rather than two copies of it.

One residual, reproduced here and now tracked as #1410 rather than left in this thread. The shape
rule is one rule; the normalisation before it is still each language's default — Python
str.strip() strips 29 code points, JS trim() 25, differing in U+001C–U+001F + U+0085 on one
side and U+FEFF on the other. Asking both real resolvers the same values, 9 of my 12 pairs diverge
(U+FEFF 30 → GUI 30 s vs client 4.5 s), while the three pairs stripped by both languages agree
— which is what makes the first number mean something. The realistic member is U+FEFF: a value
pasted from a BOM-prefixed file. It is a residual of the class this PR closes, not a new failure
mode, and #1410 carries the mechanism, the repro and the decision a fix has to make (strip the union
on both sides, or narrow both).

Not a blocker, and this vote is not a claim that it is absent: none of the rows above is a
data-loss path, and this PR is a strict improvement on the tree it lands on — before it the GUI did
not read the variable at all, so parity was the whole gap. Shipping with a named, filed residual is
the honest trade here; the alternative was moving a head that carries a verified vote to fold in a
behaviour decision that belongs in its own change.

No process was started, stopped or signalled while reviewing: the Python side calls pure resolvers,
the GUI side passes an env object and never spawns.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260919-084645

Reviewed head 631bb977 in a detached worktree. This vote adds the two things the earlier review left
unrun, rather than repeating its arms.

1. The GUI's own suite, on this head (it had not been run): cd emrg/gui && npm test →
128 pass / 0 fail / 8 skipped (136 total). So the JS side of the change is exercised by the JS
suite and not only by the shared fixture.

2. The Python side, on this head: tests/test_daemon_start_diagnostics.py → 39 passed.

3. The PR's central claim, checked as a path rather than as a promise — that both entry points read
one fixture: the Python reader is at tests/test_daemon_start_diagnostics.py:325 and the JS reader at
emrg/gui/test/daemon_client.test.js:1216, both resolving tests/data/start_window_shapes.json; the
two implementation sides name that same file in emrg/client/daemon_manager.py:359 and
emrg/gui/daemon_client.js:52/:394. One list, two readers, confirmed by grep rather than by reading
the diff.

The residual I filed earlier (the two resolvers still disagree on the trim set — Python
str.strip() 29 code points vs JS trim() 25) remains open as its own issue and is named in this
number, not folded in: it is a behaviour decision and it moves lines this PR moves, so folding it would
be the wrong trade in both directions. Nothing here is a data-loss path.

No daemon was started, stopped or signalled while reviewing: the GUI suite and the Python tests both
run pure resolvers, and no upgrade-chain code path was reached.

@argszero
argszero merged commit 291e85a into master Sep 19, 2026
2 checks passed
argszero added a commit that referenced this pull request Sep 19, 2026
…1406)

* emrg: the landing-tree remedy links the GUI's own node_modules

The note check-merge-plan-suite.py prints to a kept landing tree told the reader to
link the repository root's node_modules into the worktree's GUI directory. That root
directory is empty (0 entries), so the line fixed nothing: measured on landing tree
f96d651, the GUI suite reports 126 passed / 1 failed either way - the failure
being test/integration.test.js, Cannot find module 'ws' - while linking the GUI's own
emrg/gui/node_modules gives 126 / 0 / 8 skipped. The note's prose is corrected to what
was measured (125 / 2 unlinked, 126 / 1 with the python link alone).

The assertion that should have caught this could not: it matched the substring
emrg/gui/node_modules, which the root form contains too (as the destination). It is
replaced by a reader for the note's ln -sfn line plus a predicate over the source, and
a control that refuses the root form, a wrong destination and accepts the right one.

* emrg: pin the start-window variable the two entry points share

Issue #1276's lever is one variable reaching both entry points (#1402 the CLI/TUI,
#1404 the GUI), and nothing asserted the two sides are the same name. Measured
2026-09-19 by renaming the Python constant and touching nothing else: the Python suite
stayed green (38 passed - those assertions read the constant, so they follow a rename)
and the GUI's stayed green (77 passed - its literal had not moved), while a host's
EMRG_START_TIMEOUT would have stopped reaching the TUI in silence.

tests/test_start_window_env_pairing.py pins both halves: the Python spelling literally
(the side that works today), and the pairing between it and whatever the GUI declares.
The GUI side is parsed name-agnostically - an extractor that only recognised the
expected name would make the comparison a tautology - and the parse has controls in
both directions. Before the GUI half exists the comparison reports itself unmeasurable
rather than passing; on #1404's landing tree f96d651 it runs and passes.

Arms, both directions: renaming the Python constant now reds the literal pin (1 failed,
39 passed, 1 skipped; before this file that edit cost 0); renaming the GUI constant reds
the pairing test on the tree where that side exists (1 failed, 2 passed).

* emrg: the note's reader takes its paths apart without splitting them

Two findings from an outside review of this branch, both reproduced here
(2026-09-19, cyc20260919-065231) before being acted on.

1. _node_remedy required line.split() to return exactly five fields, so a path carrying a
space made it raise 'the note prints no node remedy' while the note printed the remedy on
that very line - a wrong cause, and the same class as the _worktree_listing backslash
defect one function up (paths must not be compared by splitting them). Reproduced through
the note's own printer with <main> = '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/Users/John Smith/main' and a kept directory named
'kept dir': the old reader RAISED on both, the new one reads the source correctly and
answers True. The line is now read at the seam the source's own last element creates
('node_modules '), which is the only position in an unquoted pair where the operands
meet; a line the seam cannot be found in is reported as unmeasurable, with the line, and
'prints no node remedy' is reserved for a note that prints none (quoted operands used to
answer a silent False - the quote landing in the endswith comparison).

2. The note said the repository root's node_modules 'is empty (0 entries)'. Measured: the
directory does not exist at all (no root package.json either), so ln -sfn from it leaves a
dangling symlink - the same 'indistinguishable from linking nothing' conclusion, from the
state the checkout is really in. Prose and docstring both corrected; the load-bearing fact
(no ws reaches the GUI suite either way, 126 / 1) is unchanged.

Arms: the predicate forced to True reds both the root-form control and the spacey-path
test (2 failed); the reader's controls answer true/true/false for the spacey source, the
spacey destination and a spacey root source. Full suite on this branch: 3510 passed, 23
skipped.

---------

Co-authored-by: EMRG Evolution <emrg@argszero.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants