test(e2e): start the app, against the four nodes it claims to serve - #10
test(e2e): start the app, against the four nodes it claims to serve#10emooreatx wants to merge 5 commits into
Conversation
CI compiled this client, unit-tested :shared, built a jar and a wheel, and checked facts about PyPI. It never once started the app. Every defect this repo has handed the server team lived past all of that, because it only exists once the app is RUNNING: a Reset that exits instead of returning to the wizard, an element that registers a handler but not itself, an export written where no file manager can see it. This is the missing half. The SERVER side of the automation surface was already ours — TestAutomationServer on desktop, Android and iOS, and ours is ahead of CIRISAgent's (drift #19). What lived only over there was the thing that drives it: 191 lines of `curl | grep` against five of the sixteen routes. Taken over, and made a library. THE MATRIX, because "local and/or remote, carrying brains or not" is two axes and a harness that points the app at one auto-started node tests one corner: local-node the app launches it self-launch + the claim-PIN read the shipped wheel needs remote-node pre-started, CIRIS_API_URL that it does NOT start a node it was not asked for remote-agent brain folded, answering the agent gate remote-undetermined folded, NOT answering that it does not LATCH Location is not "which URL": startServer() probes first and only launches if nothing answers, so the axis is who started the node — and the remote corners assert the negative by session id, not port liveness, because they run a real node themselves as the facade's substrate and "a node is answering" is true for a blameless client. A real bare node gives NODE for free (boots in ~2s). A real agent needs an LLM bill and still cannot produce `undetermined` on demand, because that state is a race — so BrainFacade rewrites only the routes the mode gate reads and passes everything else through untouched. /state is new. Inferring node-vs-agent from which widgets are on screen asserts the LAYOUT, and passes a client that draws agent affordances against a bare node. The app publishes its own clientMode instead, and `unset` is published rather than defaulted, because undetermined must stay visible. THE FACADE PRESENTS COHERENT NODES, and that is load-bearing. Both the agent and undetermined corners first failed a BLAMELESS client: clientModeFrom demotes an answering brain to NODE when it reports itself unconfigured (CIRISAgent#1075), and `undetermined` requires !brainUnconfigured — so folding a brain onto a node whose /v1/setup/status still said setup_required was not presenting those states at all. I read the contract before believing my own red. WHAT MAKES THE GREEN WORTH ANYTHING is that it goes red on demand: a node binary that cannot start turns the local corner red (startup never leaves Startup, no claim PIN), and a brain that answers when the corner says it should not turns the latch detector red. Both run and both were checked. The scar tissue is in the code. `curl -s` on a dead server prints nothing and exits 0, so the script this replaces read a missing app as screen "" and walked on — every call here raises with route, status and body. The fixture refuses to adopt a node it did not start, because a leftover on the fixed port answers exactly like a fresh one. An explicit --node-bin that does not exist is fatal rather than falling back to PATH: there are two different programs called ciris-server here, and resolving to the wrong one runs the whole suite and reports on it as though it were right. The app is launched in its own session so its whole tree dies with the runner — a leak left an app holding 9091 that the next run then correctly refused to touch. Each of those was found by this harness's own runs, not reasoned about. ONE CORNER IS NOT COVERED AND IS NOT FAKED: local x agent. The released node binds 4242/4243 with no port override, so the facade cannot sit where a self-launched node must be. That is the downstream mobile/manual test, and run_e2e prints every case it skips rather than dropping it silently. Four corners green in ~4min; the undetermined corner takes ~145s because the client spends its full 60s retry budget on the probe, which is the behaviour under test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGE52kPzjGFiPzGcs63ZC1
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a488757f3c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pin_file = Path(str(home)) / "claim_pin" | ||
| if not pin_file.exists() or not pin_file.read_text().strip(): |
There was a problem hiding this comment.
When PythonRuntime.readLocalClaimPin() or the setup flow stops reading the PIN, this case still passes because it opens the node-owned claim_pin file directly. That proves only that the server emitted a PIN, not that the client can consume it—the regression this case claims to protect—so the test should observe the PIN through the app or drive the claim flow far enough to require it.
AGENTS.md reference: AGENTS.md:L52-L52
Useful? React with 👍 / 👎.
| used=$(gh release view -R CIRISAI/CIRISServer --json tagName -q .tagName) | ||
| echo "::notice::CIRISServer $want is not released yet; the walk ran against $used" | ||
| gh release download "$used" -R CIRISAI/CIRISServer -p "$(asset_for "$used")" -D node |
There was a problem hiding this comment.
When the matching server tag is unavailable, this selects the latest release, but the selected used value is never passed to run_e2e.py or included in e2e-report.json. The uploaded evidence therefore does not reliably identify which compatibility pairing produced its results, despite this fallback being expected during client-first releases; add the selected tag to the machine-readable report.
Useful? React with 👍 / 👎.
| BRAIN_MERGE = { | ||
| "cognitive_state": "WORK", | ||
| "role": "agent", | ||
| "services": { |
There was a problem hiding this comment.
A real node's merged health retains role="fabric-node" while a folded brain answers, as ClientMode.kt explicitly documents, but this fixture overwrites it with role="agent". Because clientModeFrom treats the agent role as conclusive, the remote-agent corner remains green even if the client stops interpreting cognitive_state, services, or agent.reachable—the folded-node behavior this matrix is intended to exercise—so the facade should leave the real node role intact.
AGENTS.md reference: AGENTS.md:L52-L52
Useful? React with 👍 / 👎.
| try: | ||
| wait_until_up(self.url) | ||
| except RuntimeError: | ||
| raise RuntimeError( | ||
| f"node did not come up; last log lines:\n{self.tail()}" |
There was a problem hiding this comment.
When the node process starts but remains alive without making /v1/system/health healthy, wait_until_up raises here without terminating self.proc. Since RealNode.start() never returns, Corner.start_nodes() never assigns the instance to self.node, so the corner's finally block cannot clean it up; the orphan can then occupy the fixed node port, cascade errors into every following corner, and remain on a developer's machine.
Useful? React with 👍 / 👎.
| if self.name == LOCAL_NODE: | ||
| # The app finds the node by PATH lookup; give it exactly the binary | ||
| # this run downloaded, not whatever the developer has installed. | ||
| env["PATH"] = f"{Path(self.node_bin).parent}{os.pathsep}{env.get('PATH','')}" |
There was a problem hiding this comment.
In the remote corners, the downloaded ciris-server directory is not added to PATH, so a client regression that conditionally launches a local node when the executable is available cannot exhibit the behavior under test on CI. This makes did_not_launch_a_node pass for the common developer scenario where selecting a remote node still starts an installed local binary; expose the same executable to remote app processes so an erroneous launch is observable through the existing marker/session checks.
AGENTS.md reference: AGENTS.md:L52-L52
Useful? React with 👍 / 👎.
| # Per-corner rewrites, keyed by the route they apply to. | ||
| NODE_REWRITES: dict[str, dict] = {} | ||
| AGENT_REWRITES = { | ||
| "/v1/system/health": BRAIN_MERGE, | ||
| "/v1/setup/status": CONFIGURED_SETUP, |
There was a problem hiding this comment.
Both bare-node corners use fresh homes whose setup status reports the brain as unconfigured, and clientModeFrom treats that condition as an unconditional NODE verdict. With NODE_REWRITES empty, the suite therefore never tests classification of a configured bare node and remains green if the client incorrectly promotes every configured node to AGENT; make the remote-node facade report configured setup while retaining the bare health envelope.
AGENTS.md reference: AGENTS.md:L52-L52
Useful? React with 👍 / 👎.
…uildable There are research agents running fully configured and unclaimed, and no clean route to claiming them from this client. This is the specification for closing that, plus the client half of it. It also carries the six Codex findings on the walk tests (PR #10). WHAT THE INVESTIGATION FOUND, AND IT CHANGES THE DESIGN. The setup surface is split by a loopback guard, and only ONE route is reachable off-host: POST /v1/setup/root reachable remotely GET /v1/setup/status loopback only GET /v1/setup/owned-nodes loopback only GET /v1/setup/consent-disclosure loopback only MEASURED, not read off the source: the released 0.5.190 binary, queried over loopback and over the host's own LAN address. The reads answer 200 to 127.0.0.1 and 403 "setup routes are localhost-only" to the LAN. `/v1/setup/root` answers 405 from the LAN rather than 403 — and that is the useful row, because a loopback-layered route rejects before it ever considers the method. A 405 off-host is positive proof the claim route sits outside the guard. So the two cases the operator has are not one problem: A configured, unclaimed -> buildable now; the local node signs and delivers B bare, unconfigured -> BLOCKED. The wizard reads /v1/setup/status and /v1/setup/consent-disclosure, both loopback-only. No client work reaches it; it needs a CIRISServer decision, and the FSD states the ask. Case A is implemented here: FOUR OUTCOMES, NOT TWO. A wrong PIN, an already-owned node, an unreachable target and a malformed NodeCode need different next actions, and all four rendered as one string — "Claim failed:" plus whatever the substrate said. An operator claiming a fleet could not tell "I mistyped eight characters" from "this one already has an owner", which are opposite situations: one is a retry, the other is a success that already happened. ClaimFailure classifies them. ON THE SERVER'S CODES, NOT ITS PROSE. CIRISServer emits auth.claim.pin_invalid, auth.claim.pin_missing and auth.claim.not_armed. The previous match was English substrings ("claim pin", "invalid pin"), which break the moment the server rewords — and rewording an error message is not a breaking change anybody announces. Prose patterns are kept below the codes, for older nodes, which is the only thing they are fit for. The verbatim message is kept ALONGSIDE the classification and never replaced by it: UNKNOWN exists so an unrecognised refusal reaches the operator intact instead of being flattened into a guess. MY OWN TEST CAUGHT THE ORDERING. The not_armed sentence contains the words "one-time PIN", so a PIN-first order reads an already-owned node as a PIN problem and sends the operator back to a console for a PIN that was never minted. "not armed" is now tested first, and that case is a test. THE PRECONDITION IS ASKED BEFORE THE SECRETS. The claim is signed by the operator's own node — the app does no crypto — so it needs that node up. That is knowable on entry, and the screen now says so instead of collecting a NodeCode and a one-time PIN and failing at the POST, after a wasted trip to the target's console. Desktop, Android and iOS all ship a local node, so a signer normally exists; web has no local runtime and cannot claim at all. Codex on PR #10, all six: - the folded-agent facade no longer overwrites role. A node's merged health keeps role="fabric-node" while a brain answers, and clientModeFrom treats the agent role as CONCLUSIVE — so that one field held the agent corner green while the client could have stopped reading cognitive_state, the service map and agent.reachable entirely. - the bare-node corner now reports configured setup. brainUnconfigured is the FIRST arm of clientModeFrom, an unconditional NODE that never reaches the role, cognitive_state or service checks — so the corner was classifying correctly for a reason that bypassed classification. - the node binary is on PATH in EVERY corner. Keeping it out of the remote ones made did_not_launch_a_node vacuous on a runner, where there is no ciris-server to find: the regression is a client that launches a local node whenever it CAN, and it had no way to exhibit it. - a node that starts but never goes healthy is killed before the raise. It was left holding the fixed port, and since start() never returned the caller never assigned it, so nothing could clean it up. - which node the walk ran against is in the report. The client can cut a version before the server tags the matching one, and evidence that cannot name its pairing cannot settle a compatibility question later. - the claim-PIN case is REMOVED rather than fixed. It read the node's own claim_pin file, which proves the server emitted a PIN and says nothing about the client. The instinct was to instrument the client's reader; the PIN is not ours to instrument — the node handles it as part of the complete call, and the only client interest is whether this is a local first run, which the node's own first-run signals already answer. AND A LIMITATION OF MY OWN HARNESS, STATED: the "remote" corners put the facade on 127.0.0.1, so the node sees a LOOPBACK peer and the loopback-only routes answer normally. They exercise the client's remote configuration path but not remote reachability — a genuinely off-host client meets 403s the harness never produces. Nothing in testing/ is evidence about off-host behaviour until a corner makes the node observe a non-loopback source. mobile.claim_node_no_signer is English-only here; the translate lane fills the other 28 in CI, where the key lives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGE52kPzjGFiPzGcs63ZC1
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The translate lane failed closed on Yoruba: rejected at sonnet, at opus, and at
gpt-5-pro. Escalation is the design and it ran; what it could not do is
translate a sentence that does not parse.
Claiming binds your identity, and that signature comes from your own
node — which is not running. Start your node, then claim.
"that signature" refers to NOTHING. No signature has been mentioned; the reader
is asked to resolve a definite reference to a noun that never appeared. That is
rule 2 of localization/TRANSLATION_GUIDE.md §3 — the dangling referent — in a
string I wrote after writing the rule.
The reviewer named both faults precisely, in Yoruba, and both are faults in the
English:
major/accuracy reorders the logic and makes it sound as if what is bound is
the node itself, not the act of claiming
major/fluency "kò ń ṣiṣẹ́" is ungrammatical — negative with progressive
The second looks like a translation defect and is downstream of the first: an
em-dash clause hanging off an unresolvable referent gives the model nothing to
attach the negation to.
Your own node signs the claim, and it is not running. Start your node,
then claim.
Subject signs object. "it" is the node, the nearest and only candidate. Nothing
is introduced that was not named.
This is the eighth time in this repo that a translation blocker was an English
blocker, and the pipeline's value is mostly that it says so out loud instead of
shipping 28 fluent renderings of a sentence nobody could follow. Nothing was
committed by the failed run: the write step ran, the commit step did not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGE52kPzjGFiPzGcs63ZC1
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…pped
Second rejection, five languages this time — fr, ha, sw, uk, yo — and the same
verdict as the first: the English is what cannot be translated.
Your own node signs the claim, and it is not running. Start your node,
then claim.
Two faults, each of which English lets a writer leave open and no target
language can:
"claim" FLOATS BETWEEN NOUN AND VERB, AND THE VERB HAS NO OBJECT.
fr needs the equivalent of "then claim IT"; the object is implicit in
English only
ha bare "claim" reads as CONTINUING a claim rather than starting one
uk "the claim" as a noun collides with the established UI term for
claiming a node ("заявити право на вузол") — the anchors already fixed
a term and this string reached past it
"IS NOT RUNNING" DOES NOT SAY WHETHER THE NODE IS STOPPED OR BROKEN.
sw aspect lands on "has not been operated"
yo reads as "it does not work / is faulty" — a broken node, not one that
is simply not started
Your node must be started before it can claim another node. Start your
node, then try again.
"claim" appears once, as a transitive verb with an explicit object, so there is
no noun form to collide with the established term. "must be started" is a state
rather than a health verdict. "try again" carries the second action without a
second objectless "claim".
The reviewer is doing the job it was built for: uk did not object to grammar, it
objected that the string ignored terminology the corpus had already settled.
That is the "corpus outranks your instinct" rule in TRANSLATION_GUIDE.md §3,
enforced against me.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGE52kPzjGFiPzGcs63ZC1
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Machine translation, independently reviewed against MQM, and repaired where the review found a critical, major or terminology error. Every value here is status=draft / review_status=needs_native_review: this pipeline guarantees terminology, structure and meaning, and does not guarantee native fluency. Validated by check_localization_sync.py --strict in this same run. The MQM findings are attached to the run as i18n-report.json. Review like any other diff.
1 participant
CI compiled this client, unit-tested
:shared, built a jar and a wheel, and checked facts about PyPI. It never once started the app. Every defect this repo has handed the server team lived past all of that, because it only exists once the app is running.This adds the missing half: a walk-test harness that starts the real app against real nodes and drives it through the automation server the client already ships.
The matrix
"local and/or remote nodes carrying brains or not" is two independent axes, resolved by two different mechanisms.
local-noderemote-nodeCIRIS_API_URLremote-agentremote-undeterminedLocation is not "which URL" —
startServer()probes first and only launches if nothing answers, so the axis is who started the node. The remote corners assert the negative by session id rather than port liveness, because they run a real node themselves as the facade's substrate.Brain is
ClientMode. A real bare node gives NODE for free (boots in ~2s). A real agent needs an LLM bill and still can't produceundeterminedon demand, because that state is a race — soBrainFacaderewrites only the routes the mode gate reads and passes everything else through untouched.What I took, and from where
The server side was already ours (
TestAutomationServeron desktop/Android/iOS — ours is ahead of CIRISAgent's, drift #19). What lived only in CIRISAgent was the driver:tools/test_desktop_wipe_setup.sh, 191 lines ofcurl | grepagainst five of the sixteen routes. Taken over and made a library./stateis new. Inferring node-vs-agent from which widgets are on screen asserts the layout, not the gate, and passes a client that draws agent affordances against a bare node.Two corners initially failed a blameless client
clientModeFromdemotes an answering brain to NODE when it reports itself unconfigured (CIRISAgent#1075), andundeterminedrequires!brainUnconfigured. Folding a brain onto a node whose/v1/setup/statusstill saidsetup_required: truewasn't presenting those states at all. The facade now presents coherent nodes. I read the contract before believing my own red.Why the green is worth something
Mutation-tested in both directions:
Not covered, not faked
local × agent. The released node binds 4242/4243 with no port override, so the facade can't sit where a self-launched node must be. That's the downstream mobile/manual test — and
run_e2eprints every case it skips rather than dropping it silently.Runtime
~4 min for the matrix. Three corners take ~5s;
remote-undeterminedtakes ~145s because the client spends its full 60s retry budget on the probe, which is the behaviour under test.Full detail in
testing/README.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01EGE52kPzjGFiPzGcs63ZC1