diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 021278b06..84668066a 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -758,3 +758,4 @@ extracting `OffsetExpression`/`LimitExpression`. | `mxcli run --local`/`--hub` sits at several hundred percent CPU for hours while the app is gone; `ps -o stat` shows the runtime JVM as `Z` (zombie) under it, `curl localhost:` returns `000`, and the hub preview URL still answers | `cmd/mxcli/docker/localboot.go` (`watchExit`, `alive`, `stopProcess`), `cmd/mxcli/docker/runlocal.go` (`waitForInterruptOrExit`, `runtimeStoppedError`) | Two mechanisms. (1) **Nothing ever called `Wait()`** on the runtime process, so an exited JVM stayed an unreaped zombie — and `alive()` asked `Signal(0)`, which **succeeds on a zombie**. Measured (Linux 6.18, go1.26): proc state `Z`, `Signal(0)` → nil; after `Wait()` → "process already finished". So the liveness check reported a runtime that had terminated itself hours earlier as alive. `Signal(0)` is only a correct liveness test *because* something reaps — removing `watchExit` silently breaks that line, which is why the control is on the reaper, not on `alive()`. (2) **After boot, `run` waited on a signal and nothing else**, so a correct answer had no one asking; it now waits on the signal OR `rt.Exited()` and returns a **non-zero** error, because returning 0 after the app has gone is what let a supervisor conclude all was well. **Why it happens at all**: the local standalone runtime uses a development licence with a maximum run time and terminates *itself* (measured lifetimes 3h52m and 5h07m — not a fixed number, and shorter than a working session); `runtimeExitReason` lifts that from the runtime's own log, and reports nothing rather than guessing when it cannot tell. **Two waiters on one process deadlock**, so `stopProcess` consults the reaper's channel instead of taking its own `Wait`. The CPU spin itself was NOT reproduced and is not claimed fixed by name — it lived in the tunnel client, under a supervisor blind to its dead child; what is fixed is the state it occurred in, since mxcli now exits and takes the tunnel with it. **Generalisable**: a 200 from a tunnelled URL is not evidence the app is alive — the tunnel outlives the runtime. Reported as mxcli-formula1 FINDINGS §60 | | After `mxcli test … --local`, an app another `mxcli run --local` is serving goes blank while still answering HTTP 200 (~1.7 KB, the Mendix SPA shell); the runtime log shows `Connector: 404 - file not found for file: dist%2Findex.js` and `deployment/web/dist` is gone | `cmd/mxcli/testrunner/localapp_options.go`, `cmd/mxcli/testrunner/runner_local.go` (`localTestDeployDir`), `cmd/mxcli/testrunner/runner.go` (`checkScratchDeploymentExists`) | A local test run already used its own ports and its own `_test` database — the code comment says why, verbatim — but shared the **deployment directory**, which is the one the *browser* reads. A headless test boot does not bundle the web client, so its build left the running app serving the shell over a 404: tests pass, run keeps running, app is blank, nothing reported at either end. **Detection was not the fix**: the two processes use different ports by design, so no port check can see it, and a lock file would only turn a silent blanking into a refusal. The test boot now builds into `/.mxcli/deployment-test/` — gitignored, already where the test runtime log lives — which makes the collision impossible. Note booting a runtime against the shared directory damages it even **without** a rebuild (the packaging step removes the bundle — FINDINGS §35, `ReportLostWebClientBundle`), so "reuse the dev loop's tree read-only" is not an alternative. Consequence to wire: `--skip-build` used to mean "reuse deployment/" and now has nothing until tests have run once, so it is refused with the reason rather than failing inside the runtime boot against a path the user never chose. Reported as mxcli-formula1 FINDINGS §62 | | A widget keyword the grammar accepts is absent from `mxcli syntax page widgets`, so it is concluded not to exist and worked around at length (reported for `tabcontainer`, which cost two days and five hand-rolled pages) | `cmd/mxcli/syntax/features_page.go`, `cmd/mxcli/syntax/widget_keywords_drift_test.go` | The lesson the reporter drew — "absence from the documentation is not absence from the grammar" — is true and is a bad thing for the docs to require. `TestEveryWidgetKeywordIsInAPageSyntaxTopic` makes it false instead: it reads the `widgetTypeV3` rule out of the **committed** `.g4` (only the *generated parser* is uncommitted, and the grammar is the authority the reporter was told to consult) and fails when a keyword appears in no `page.*` topic. It found **18**, not one. Exemptions go in `documentedElsewhere` **with the topic that owns them** — layout constructs (`scrollcontainer`, `region`, `navigationtree`, `menubar`, `placeholder`) and pluggable-widget object-list keywords (`group`, `series`, `marker`, …) are not page widgets; an entry with no home is the same defect. The guard carries its own vacuity control: a keyword that does not exist must not match, and one that does must. **Do not document a keyword without running it** — probing all 18 on 11.13 found four the parser accepts and the *default engine refuses* (`statictext`, `staticimage`, `dynamicimage`, `dropdown` → "widget *pages.X not yet supported by the modelsdk engine"), two refused on both engines (`referenceselector`, `legacydatagrid`), and one whose bare form emits **CE0463** (`image`). Reported as mxcli-formula1 FINDINGS §69 | +| Every open PR goes red at once on `build-and-test` with a failure in a package none of them touched — `--- FAIL: TestSessionLog_PersistAndPrune`, "after reload+prune: 0 records, want 1" — and the same test fails on a clean checkout of `main` | A **time bomb in the test**, not a regression: the fixture pinned `base := time.Date(2026, 8, 1, ...)` against a 30-day retention window, and `NewSessionLogFile` prunes inside `load()` — *before* the test can assign `log2.now`, so the reload prune runs on the real `time.Now()` whatever clock is injected afterwards. It passed for 30 days and then failed permanently, on every branch simultaneously | `cmd/mxcli/tunnelhub/sessions_test.go` (`TestSessionLog_PersistAndPrune`), `cmd/mxcli/tunnelhub/sessions.go` (`load` → `pruneLocked` → `clock`) | **First establish it is not yours**: run the failing test on a clean `origin/main`. Several PRs failing on one unrelated test is the signature. Then make the fixture relative — `base := time.Now().UTC()` — so the record ages, not the calendar, decide the outcome; the other tests in the file keep their fixed base legitimately, because they use `NewSessionLog` and inject the clock before recording. **A date fixture is only safe where no code path reads the real clock**; the moment a constructor prunes, expires or compares against `time.Now()` before the seam is in place, an absolute date has a fuse on it. Control the repair: stub `pruneLocked` to a no-op and confirm the test still fails (2 records, want 1), or the fix is just a test that stopped testing | diff --git a/cmd/mxcli/tunnelhub/sessions_test.go b/cmd/mxcli/tunnelhub/sessions_test.go index f15480203..ed1a90691 100644 --- a/cmd/mxcli/tunnelhub/sessions_test.go +++ b/cmd/mxcli/tunnelhub/sessions_test.go @@ -24,7 +24,15 @@ func TestSessionURL(t *testing.T) { func TestSessionLog_PersistAndPrune(t *testing.T) { path := filepath.Join(t.TempDir(), "sub", "hub-sessions.json") - base := time.Date(2026, 8, 1, 12, 0, 0, 0, time.UTC) + // Relative to the real clock, unlike the fixed base the other tests here + // use, and that difference is load-bearing: NewSessionLogFile prunes inside + // load(), before a test can inject its clock, so the prune on reload runs + // against time.Now() no matter what is assigned to log2.now afterwards. With + // a hardcoded base this test passes until the wall clock drifts past the + // retention window and then fails for good -- which is what happened on + // 2026-08-31, exactly 30 days after a base of 2026-08-01 and a 30-day + // retention, on every branch at once. + base := time.Now().UTC() log, err := NewSessionLogFile(path, 30*24*time.Hour) if err != nil { @@ -42,7 +50,9 @@ func TestSessionLog_PersistAndPrune(t *testing.T) { Subdomain: "ancient", LastSeenAt: base.Add(-40 * 24 * time.Hour), }) - // Reload from disk with "now" past the old record's retention window. + // Reload from disk. The prune happens during load, on the real clock: the + // 40-day-old record is outside the 30-day window and the 1-hour-old one is + // comfortably inside it, so the outcome does not depend on when this runs. log2, err := NewSessionLogFile(path, 30*24*time.Hour) if err != nil { t.Fatalf("reload: %v", err)