Skip to content

fix(server): keep Cue skipped across serving restarts (#271) - #277

Merged
lukemaj merged 2 commits into
mainfrom
fix/271-cue-stay-down
Oct 14, 2026
Merged

lukemaj merged 2 commits into
mainfrom
fix/271-cue-stay-down

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

What: Chromeria's Cue client stays skipped after one connect failure until /health returns Cue's health document, including after the serving process restarts.

Why: The down flag lived only in memory, so rocky's overnight crash loop (#256) started each new process by calling Cue again. Bestia's long-lived process held the skip against the same dead port. A probe also treated any HTTP 200 as recovery.

So what: Merge after review/independent-sol and review/independent-grok succeed on this head. No version bump. Do not restart Cue or Chromeria for proof.

Closes #271.

Elon record

Requirements and who asked: Machine-wide skip after a connect failure until health succeeds, including across rocky's crash-loop restarts (user via #271 and contextmd#13 acceptance).

Deleted: Changing the user-message wait or the window (#248). Stopping Cue to test. A second backoff policy besides the existing probe. Treating bestia's hold as if rocky had the same process lifetime.

Bottleneck: In-memory available resets on process start, and a living process re-armed when the probe treated HTTP 200 as recovery.

Checked myself: Rocky's serving sqlite (read-only), heartbeat phase resets, crash reports, bestia's cue journal, Cue.ts before this change, and a closed-port reproduction that returns connect_failed rather than HTTP 200.

Proof

Focused tests on this worktree, node 26, npx -y pnpm@11.10.0 exec vp test run of Cue.test.ts, ServingState.test.ts, and ContextmdRoute.test.ts: 92 passed (12, 55, 25). The route test copies the sqlite after the first connect_failed and starts a second server that logs not_called with no further transport call. Exact-key injection still completes while Cue is down.

Upstream pingdotgg/t3code has no CueClient (code search, 0 hits). These files are fork-owned and are not listed in scripts/fork-upstream-edits.txt.

No version bump: the task and chromeria AGENTS.md forbid one. release-check is not the gate for this PR.

Website impact: none. This is internal serving behavior with no public setup or capability change.

Limit

The health-document check (ok: true and a non-empty adapter) stops a bare HTTP 200. It does not stop a real Cue health body. Some overnight re-arms sat inside one stable heartbeat phase, so they were not process restarts. The old code can set available back to true only from a probe that saw HTTP 200, and contextmd-cue.service stayed stopped from 2026-10-13 19:52:42Z until 2026-10-14 06:21:55Z. A closed port on this machine returns ECONNREFUSED, not 200. Those in-process re-arms are recorded, and this change does not claim to explain a genuine Cue health document.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M labels Oct 14, 2026
@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Agent Observer had no sessions in scope for this task after sync, so there is no cost comment. The gap is measurement, not the change.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Clean independent review of PR #277 at bf406f7f0359f39a515ac7084e51ae6b74120679; no blocking or non-blocking findings.
Why: The persisted per-URL skip, restart initialization, and health-document recovery implement the requested availability behavior without propagating listener exceptions into a moment.
So what: This head passes review/independent-sol; merge remains subject to the other required review and human approval.

Reviewed base 6d167d4542982f1bda8532a6d0fc2886d0c4fe8c, the complete gh pr diff 277 --repo toolboxmd/chromeria, the files at the exact head, and the serving/probe consumers. Worktree HEAD and live PR head both matched the requested SHA.

Blocking findings: none.
Non-blocking findings: none.

Acceptance checked by source inspection:

  • Only a score connect_failed marks the client down. The synchronous availability listener writes cue_down, and a newly constructed serving client reads that persisted URL before scoring. Ordinary stale-reservation cleanup does not remove this state.
  • The existing background probe remains active for unavailable clients. Recovery requires GET /health, HTTP 200, parsed JSON with ok === true and a non-empty string adapter; malformed documents and failed probes retain the skip.
  • Transport timeouts do not change availability. Availability-listener exceptions are caught after the in-memory transition, so they do not turn a connect failure into a failed local moment. Persistence is explicitly best-effort on storage errors.
  • Cue remains optional to local selection/serving. The existing route test asserts exact-key injection during a closed-port outage, and the new restart route test seeds a second serving instance with the marked sqlite and observes subsequent not_called moments. Unit tests separately cover health recovery and timeout availability.
  • The additive table is created on database open, with no version bump. Changed paths are fork-owned and absent from scripts/fork-upstream-edits.txt.

The overnight explanation correctly separates rocky's process-lifetime resets from bestia's long-lived skip and retains the unexplained in-phase re-arms as a limit. I reviewed the issue's evidence and explanation; I did not independently query either live host. A genuine Cue health document permits recovery under the stated contract, and I found no defect requiring a second consecutive-probe policy.

No tests were run, as requested: no finding required execution. No repository files were changed, and neither Chromeria nor Cue was stopped or restarted.

Reviewer: gpt-6.1-sol, medium reasoning effort, Codex harness.

@lukemaj
lukemaj force-pushed the fix/271-cue-stay-down branch from bf406f7 to 8d75e65 Compare October 14, 2026 07:56
@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main after #274. The only conflict was the test helper return: it keeps both release and sqlitePath. Cue behavior is unchanged. New head 8d75e65ace98d7ea018e26701c4156c98b98a7f9. Focused tests: 94 passed (Cue 12, ServingState 56, ContextmdRoute 26).

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent Sol round 2 review of 8d75e65ace98d7ea018e26701c4156c98b98a7f9 is clean, with no blocking or non-blocking findings.
Why: The new head preserves the persisted Cue skip and health-document recovery, and the rebase retains the reservation-release path.
So what: review/independent-sol can succeed on this exact head; merge remains subject to the other required review and human approval.

Reviewed the full gh pr diff 277 --repo toolboxmd/chromeria and exact-SHA source and surrounding consumers. Worktree HEAD and live PR head both matched the requested SHA before review and again before this verdict.

  • A score connect failure transitions availability to false and persists the URL in cue_down. Subsequent score/start calls skip; a new serving instance initializes from the persisted marker.
  • Recovery requires HTTP 200 with JSON ok: true and a non-empty string adapter; a bare 200 does not clear the marker. A genuine Cue health document still resumes, as accepted.
  • Timeout does not set availability false. The local decision and exact-key lane remain reachable while Cue is skipped; the route test asserts the exact-key reply under that condition.
  • The rebased serve() return includes both release and sqlitePath. The authenticated release route, service, state operation, and once-only release/settle assertions remain present. Cue.ts and Cue.test.ts are unchanged from the prior candidate; the persisted-state integration was inspected again on this head.

Blocking findings: none. Non-blocking findings: none.

No tests run: no finding depended on execution, per the requested proof limit. No repository edits, checkouts, or service restarts performed.

Model and harness: gpt-6.1-sol, Codex in T3 Code.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Clean independent review of PR #277 at 8d75e65ace98d7ea018e26701c4156c98b98a7f9; no blocking or non-blocking findings.
Why: One score connect failure skips Cue until /health is HTTP 200 with ok: true and a non-empty adapter, and that skip is stored in cue_down so the next process starts skipped.
So what: review/independent-grok succeeds on this head. Merge still needs human approval.

Reviewed base a68e39462f10c027d79f256856d082dc967a5676, the full gh pr diff 277, and the files at this SHA. Worktree HEAD and the live PR head matched before review and again before this verdict. Tests were not re-run; no finding depended on a run.

Blocking findings: none.
Non-blocking findings: none.

  • request() calls setAvailable(false) only for transport connect_failed. A timeout, overload, rejection, or error leaves availability unchanged.
  • The availability listener writes or deletes cue_down only on a real transition. A listener exception is swallowed after the in-memory update, so a disk error does not fail the moment.
  • A new serving client starts from !state.cueDown(url). CREATE TABLE IF NOT EXISTS cue_down runs on open, including an older state file.
  • probe() marks the client up only for GET /health, HTTP 200, and JSON with ok === true and a non-empty string adapter. A bare 200 stays down. ContextMD's handler returns {"ok": true, "adapter": variant.id, "threshold": ...}, which matches. A genuine health document still resumes, as accepted.
  • The first moment after a restart skips without a score call. The parked probe then covers clients that are already down.
  • Exact-key injection stays in state.decide after Cue returns. The existing route test still expects rule a while the statuses are connect_failed, not_called, not_called. The restart test copies the sqlite and expects the next process to log not_called.
  • The rebased serve() helper returns both release and sqlitePath. The release route assertions and releaseReservation are still present.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 6d167d4 · PR result: 69b4eef · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@lukemaj
lukemaj force-pushed the fix/271-cue-stay-down branch from 8d75e65 to 69b4eef Compare October 14, 2026 08:10
@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Typecheck on 8d75e65 failed in our code: ContextmdServing.ts called Date.now(), which is error TS377066 (effect(globalDate)). Main's typecheck on a68e394 passed. The skip now uses Clock.currentTimeMillisUnsafe().

Head is 69b4eefc3dae0cfc2ccf924262be42a7b627de7b, rebased onto 51ab123 (#272). That commit does not touch contextmd. Server tsc --noEmit exited 0. Cue.test.ts and ContextmdRoute.test.ts: 38 passed.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Round 3 independent review is clean on 69b4eef.
Why: The inspected availability transitions, persisted skip, health validation, and Effect clock change satisfy the requested acceptance without a supported defect.
So what: No review fixes are required; this verdict applies only to this exact head and grants no merge authority.

Blocking findings: none.
Non-blocking findings: none.

Verified the worktree HEAD and live PR head both equal 69b4eef, with base 51ab123. Read the complete gh pr diff 277 --repo toolboxmd/chromeria, the changed files at that SHA, and the surrounding request, probe, database initialization, and exact-key decision paths. Independently checked the delta from 8d75e65; the contextmd delta replaces the global Date clock with the captured Effect Clock service.

Connect failures synchronously record the URL as down; a new client reads that state. Subsequent score/start requests skip transport while down. Recovery requires HTTP 200, ok === true, and a non-empty string adapter. Timeout replies do not change availability. Cue replies remain separate from local exact-key candidates. ContextmdServing.ts contains no Date.now() and passes clock.currentTimeMillisUnsafe() to the Cue client and persisted failure timestamp.

Inspected the focused tests for restart persistence, valid health recovery, timeout isolation, and exact-key injection while down. No tests were run in this review because no finding required execution. Author-reported test/typecheck results were not treated as independently executed proof. The accepted genuine-health-document recovery limit remains unchanged. No files or services were modified, and no service was stopped or restarted.

Reviewer: gpt-6.1-sol, medium reasoning effort, Codex harness.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Independent review, round 3. Head 69b4eefc3dae0cfc2ccf924262be42a7b627de7b (worktree and live PR head match).

Blocking: none.
Non-blocking: none.

Checked on this head:

  • A connect failure is the only transition to down (setAvailable(false)). A transport timeout, a caller wait that returns timeout, overload, a 4xx, and a thrown transport leave the client available.
  • A probe marks the service up only for HTTP 200 with ok: true and a non-empty string adapter. HTTP 200 with body ok stays down.
  • The skip is the cue_down row. SCHEMA runs on every open, so an existing state file gains the table. The next process starts that URL unavailable and does not call /score until a health document clears the row.
  • Exact-key injection does not depend on Cue being up. cueFires adds candidates only when the reply status is ok. The in-process route test still expects the exact-key body while later moments are not_called.
  • ContextmdServing.ts takes time from clock.currentTimeMillisUnsafe() (Effect 4.0.1 Clock). That file has no Date.now().

No version bump. Tests were not re-run; no finding depended on a run.

@lukemaj
lukemaj merged commit 918e78f into main Oct 14, 2026
28 checks passed
@lukemaj
lukemaj deleted the fix/271-cue-stay-down branch October 14, 2026 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cue client on rocky retries the service after marking it down

1 participant