Skip to content

Fix run --hub: re-register with the hub on heartbeat 404 (findings #16) - #27

Merged
ako merged 1 commit into
mainfrom
fix/hub-reregister
Jul 25, 2026
Merged

Fix run --hub: re-register with the hub on heartbeat 404 (findings #16)#27
ako merged 1 commit into
mainfrom
fix/hub-reregister

Conversation

@ako

@ako ako commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Batch 3 of the sudoku findings: the hub re-registration bug (#16).

The bug

mxcli run --hub sent its hub registration exactly once, at startup. The embedded chisel transport reconnects forever on its own — but nothing re-sent /api/register. So when the hub restarts and loses its in-memory registry:

  • /api/status heartbeats return 404 "unknown token", and nothing recreated the entry;
  • the tunnel still looks healthy locally (chisel is connected), so the local side reports success;
  • but the hub lists no session and the public URL is dead — the classic "page loads from cache but every action fails / /xas/ unreachable."

The invisibility (local success, remote dead) is what made it hard to diagnose.

The fix

The heartbeat is now self-healing:

  • postToken returns the HTTP status; on 404/401 the heartbeat re-registers in place (reRegister() re-POSTs /api/register with the stored identity).
  • Same identity usually re-registers to the same subdomain/reverse port, so the still-connected tunnel immediately works again — no restart needed.
  • When the re-register lands on a different reverse port (port pool reset after a hub restart with multiple previews), an onReRegister callback restarts the tunnel to the new port. runlocal guards the tunnel handle with a mutex since the callback runs on the heartbeat goroutine.
  • HubRegistration now retains the register inputs (secret/meta/appPort) so the heartbeat can re-register without extra plumbing.

Test

TestHeartbeatReRegistersAfterHubForgets — a fake hub that 404s /api/status and returns a new port on re-register; asserts the token/port update and that the tunnel-restart callback fires. Passes in ~1s.

(The only failing test in the package is the pre-existing TestServeIntegration — a MxBuild 11.6.3-vs-11.12.1 version mismatch, unrelated to this change.)

🤖 Generated with Claude Code

Generated by Claude Code

@claude
Fix run --hub: re-register with the hub on heartbeat 404 (findings #16)
aad1af2 
The embedded chisel transport reconnects forever, but the hub REGISTRATION was
sent exactly once at startup. When the hub restarts and loses its in-memory
registry, /api/status returns 404 "unknown token" and nothing recreated the
entry — so the tunnel looked healthy locally while the hub listed no session and
the public URL was dead (static assets cached, /xas/ unreachable). The failure
was invisible because the local side reported success.

The heartbeat is now self-healing: postToken returns the HTTP status, and on a
404/401 the heartbeat re-registers in place (same identity → usually the same
subdomain/port, so the existing tunnel immediately works again). When the
re-register lands on a different reverse port, an onReRegister callback restarts
the tunnel to the new port (runlocal guards the tunnel handle with a mutex since
the callback runs on the heartbeat goroutine).

HubRegistration now keeps the register inputs (secret/meta/appPort) so the
heartbeat can re-POST /api/register without threading them through separately.

Test: TestHeartbeatReRegistersAfterHubForgets (fake hub that 404s /api/status
and hands back a new port on re-register; asserts the token/port update and the
callback fire). The only failing docker test is the pre-existing
TestServeIntegration (MxBuild version mismatch), unrelated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
@ako
ako merged commit cd14f18 into main Jul 25, 2026
3 checks passed
ako pushed a commit that referenced this pull request Jul 28, 2026
@claude
feat(mdl): check-time advisories for Mendix expression/XPath/layout g…
537137b 
…otchas

Three check-time hints for constructs that pass `mxcli check` but surprise at
build or render time (ledger findings #21/#25/#27):

- MDL046: dateTime()/dateTimeUTC() with a non-literal argument (CE0117 — these
  build from hardcoded constants only). Hint: step off a literal anchor with
  addDays()/addMonths(), which take variables.
- MDL047: an association compared to `empty` in a retrieve constraint
  (`[Module.Assoc = empty]` → CE0161 — `= empty` tests attributes, not
  associations). Hint: `[not(Assoc/Target)]`. A bare attribute and an
  attribute-over-association test are correctly not flagged.
- MDL-WIDGET15 (info): two or more adjacent dynamictext siblings, which Mendix
  renders inline (concatenated, no separator) regardless of RenderMode. Info
  severity — advisory only, never fails the build.

Finding #20 (division operand typing) is covered by the existing MDL041
(div→Decimal) and the new MDL045 (`/`-as-division) rather than a separate rule
that would contradict CLAUDE.md's documented "integer div yields Decimal".

Tests: TestValidateMicroflow_DateTimeLiterals, _XPathAssociationEmpty,
TestValidateConsecutiveDynamicText. Repros in mdl-examples/bug-tests/. Skills +
symptom table updated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
@ako ako mentioned this pull request Jul 28, 2026
Merged
ako pushed a commit that referenced this pull request Jul 28, 2026
@claude
fix(mdl): close verification-round gaps in ledger checks (#17/#25/#27)
f297447 
Re-test of PR #52 by the ledger project surfaced three gaps:

- #17 (MDL045): the variable/variable division form `$Dec / $Dec2` was not
  caught — it parses as a member-access path, so the BinaryExpr walk missed it.
  The visitor now narrowly preserves source when a `/` is immediately followed by
  `$` (a real association path never has `$` after `/`, so legit navigation is
  untouched), and MDL045 flags a source-preserved AttributePathExpr whose `/ $`
  source matches (`exprIsSlashDollarDivision`). The `$` guard on the RHS is the
  reliable division-vs-navigation discriminator the tester suggested.

- #25 (MDL047): the check only saw microflow `retrieve` constraints, so an
  association `= empty` in a page/widget datasource where-clause slipped through.
  Now `validateDatasourceXPathAssociationEmpty` inspects `DataSourceV3.Where` on
  every widget, sharing the detection via `xpathAssociationEmptyMatches`.

- #27 (MDL-WIDGET15): the advisory false-positived on heading+subtitle pairs.
  Only `Text`/unset RenderMode is inline; H1–H6/Paragraph are block-level and do
  not concatenate. `inlineDynamicText` now excludes block modes, so a heading
  breaks the run.

Tests updated (variable/variable division, page-datasource MDL047, heading
exclusion). Repros: ledger-17-slash-division.fail.mdl (+ var/var form),
new ledger-25-page-datasource-assoc-empty.fail.mdl. Symptom table refreshed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
ako pushed a commit that referenced this pull request Jul 28, 2026
@claude
fix(mdl): MDL-WIDGET15 — Paragraph renders inline, not block (#29)
c5c724b 
Verification round found that the #27 narrowing wrongly treated `Paragraph`
render mode as block-level. On Mendix 11.12.1 + Atlas a Paragraph-mode
dynamictext renders as an inline `<span>`, so two adjacent Paragraph widgets fuse
(`PARA-ONEPARA-TWO`) exactly like Text-mode ones — and the advisory was silently
missing them.

`inlineDynamicText` now treats ONLY H1–H6 as block-level (`headingRenderModeRe`);
Text/unset and Paragraph are both inline. The advisory message is corrected to
recommend a heading RenderMode (not Paragraph, which does not fix the problem).

Tests updated: Paragraph+Paragraph and Paragraph+Text now flagged, heading pairs
still excluded. Repro ledger-27 extended to show all three cases. Symptom table
notes the lesson: verify Mendix render behavior empirically — a name like
"Paragraph" does not imply `display: block`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
@ako ako mentioned this pull request Jul 30, 2026
Merged
ako pushed a commit that referenced this pull request Jul 30, 2026
@claude
fix(grammar): allow a /** */ doc comment between alter-entity add-att…
06910e3 
…ribute clauses (TraceOps #27)

A `/** … */` doc comment written between two `alter entity … add attribute`
clauses (before the ADD keyword) was a parse error ("no viable alternative at
input '/**'"): alterEntityAction accepted a doc comment only INSIDE an
attributeDefinition. `--` line comments are not an equivalent workaround — they
are discarded by the lexer, whereas a `/** */` doc comment is persisted as the
attribute's Mendix documentation, so rewriting to `--` silently drops the
documentation the author meant to store.

alterEntityAction now accepts an optional leading docComment on ADD ATTRIBUTE /
ADD COLUMN, and ExitAlterEntityAction attaches it as the added attribute's
documentation when the attributeDefinition itself has none. Verified against
mxbuild 11.12.1: the script builds clean and DESCRIBE ENTITY round-trips the doc
comment on the second column.

`make grammar` regenerates the parser (generated files are not committed).
Test TestAlterEntityAddAttributeDocComment; repro
mdl-examples/bug-tests/traceops-27-doc-comment-between-clauses.mdl.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ako @claude