[jsavin]

Fixes #891.

Headless Frontier no longer launches a host GUI browser. The first-run onboarding intent is preserved — only the surface moves.

Why this is not "suppress the browser launch"

Legacy Frontier launched the owner's browser at the setupFrontier page to collect configuration (site name, admin credentials) from potentially non-technical server owners. That is a required product feature, and it is what the current goal means by "completes first-run."

The wrong part is narrow: a headless/server/CI process reaching into a host GUI session. There is no human at that machine's desktop, and the daemon host is frequently not the operator's workstation. So the headless default now logs the URL for a remote operator and records it for flows and tests; the launch itself is preserved behind an explicit opt-in and is unchanged in GUI builds.

Root cause: headless was the MORE aggressive path

webBrowser.openURL already branched on system.environment.isHeadless — but that branch was never a no-exec branch:

if system.environment.isHeadless {
	if not string.isValidUrl (s) { scriptError (...) };
	try { sys.openUrl (s); return (true)};
	msg ("Open this URL in your browser: " + s);
	return (false)};

sys.openUrl is the kernel verb that double-forks and calls execlp("open", ...). It succeeds, so the msg(...) fallback never runs and a browser opens. Headless routed through open(1) while interactive routed through an AppleEvent — the headless path was the more aggressive of the two. That inversion is the bug.

Trigger chain (this is how a browser actually launched during test cycles):

1. finishInstall.ut:227 sets system.temp.installer.urlToOpen = setupUrl (its own openUrl call at :228 is commented out)
2. startupScript.ut:252 calls webBrowser.openUrl (system.temp.installer.urlToOpen) at the next startup phase
3. → sys.openUrl → execlp("open", ...) → browser opens on the host desktop

Where the guard goes, and a near-miss worth recording

The guard is at the sys.openUrl kernel verb — the single choke point every caller funnels through (webBrowser.openURL, trialVersionCheck, ftpTable, Frontier.tools.commands.openUrl, startupScript). A guard here cannot be bypassed by a script calling sys.openUrl directly, and it requires zero ODB edits, so no Virgin.root binary edit and no re-run of the #881/#892 repair risk.

The near-miss: the exec path was first traced to Common/source/shellsysverbs.c by reading the code. That file is not linked into frontier-cli (grep shellsysverbs frontier-cli/Makefile → no matches). The CLI's real implementation is tests/headless_sys_verbs.c (case sysv_openurl), pulled in via tests/headless_verbs.mk:47 → frontier-cli/Makefile:240 → :255 → :266. A fix landing only in shellsysverbs.c would have compiled, reviewed cleanly, and done nothing.

What caught it: the red test ran against the built binary and captured a real exec —

FAIL: default performs NO exec
  Exec recorded: open http://127.0.0.1:5336/setupFrontier

That line is the TU-identity proof. Both copies are now changed in lockstep, each carrying a TWIN: comment naming the other and stating which one is linked. Duplication tracked in #893.

Behavior (headless build)

--browser	Behavior
absent (default)	No exec. Log the URL, set system.temp.Frontier.pendingBrowserUrl, return true.
default	Identical to absent.
agent-browser	Unchanged — execs agent-browser (automated verification path).
allow-gui	New. Restores the real open(1) / xdg-open launch for a local desktop-style session.
anything else	Rejected with a scriptError, as before.

The opt-in is a new value on the existing --browser flag rather than a separate --allow-gui-browser flag: one flag gives one answer to "what does openUrl do here," where two flags could contradict each other. Usage text in frontier-cli/main.c documents all three modes.

GUI builds are unaffected by construction — the whole block sits inside FRONTIER_HEADLESS.

Marker semantics

system.temp.Frontier.pendingBrowserUrl holds the last URL that wanted a browser: written on every no-exec call, overwriting, with no URL sniffing. Unit B (the firstRootRun flag flip) asserts this equals the setup URL, which is a stronger assertion than a boolean would be. It is stored as a heap string rather than via the bigstring assign helpers, since a URL can exceed the 255-byte Pascal-string limit. A rejected URL (for example the empty string) never writes a marker.

Also fixed: a latent wrong Pascal length byte

shellsysverbs.c declared \x45 for a 76-character rejection message. Because that TU is not compiled by the CLI build, the headless static_assert never saw it. My twin edit initially repeated the mistake; the headless build caught mine, and I then verified the twin arithmetically. Both are now \x4c. The pre-commit hook confirms 1026/1026 Pascal lengths correct across 50 files.

Tests

tests/openurl_headless_guard_test.sh (new, wired in as test-openurl-guard in test-all and .PHONY).

Hard rule: no test may launch a host GUI browser. Exec assertions use PATH-shadowing stub launchers named open, xdg-open, and agent-browser that record their argv to a file. Since execlp resolves through PATH, the stub intercepts and the real browser is never reached — this is what lets the suite assert positively that the opt-in does exec.

Coverage: default returns true / performs no exec / sets the marker; explicit default behaves identically; marker overwrites per call; allow-gui execs with the URL and leaves the marker unset; agent-browser still execs; unknown values rejected with no exec; empty URL errors before any fork.

• Red (pre-fix): 8 passed / 7 failed, including the Exec recorded: open ... capture above.
• Green (post-fix): 15 passed / 0 failed.

Four integration cases added to sys_processor_tests.yaml, and two stale skip: "sys.openUrl not implemented" entries cleared — the verb is implemented, and those skips were hiding this path. One marker test explicitly clears the marker first, since sequential tests share one session (#852).

Suite evidence

	This branch	Baseline 2b56ce868
Integration total	2491	2488
Passed	2313	2308
Skipped	161	163
Failed	0	0
Unit	exit 0	exit 0

Delta is exactly this change: +3 net tests, −2 stale skips.

Every anomaly was attributed against a purpose-built pristine baseline worktree (since removed) rather than assumed:

• 3 sys.appisrunning failures — reproduce identically on pristine 2b56ce868. Pre-existing, untouched.
• One test_callback_infrastructure SIGSEGV on the first unit run — this is Latent: in-memory hash node takes on-disk resolution path -> NULL deref (dbgeteof via hashresolvevalue) #828 verbatim, which names this exact binary at a 30–80% load-dependent rate. Passed standalone 14/14 and on a clean re-run (exit 0). Not introduced here, but recorded since it fired during this window.

Follow-ups filed

• Duplicated sys verb implementations across linked/unlinked TUs (openUrl, unixShellCommand) -- wrong-file edits silently no-op #893 — duplicated sys verb implementations across linked/unlinked translation units (openUrl plus the unixShellCommand twin); the link-time ambiguity that produced the near-miss above. The TWIN: comments added here are the stopgap and should be removed when that lands.
• sys.unixShellCommand can exec a host GUI browser past the #891 headless openUrl guard -- decision needed #894 — sys.unixShellCommand (popen → /bin/sh, in the CLI build) can exec a launcher past this guard. Deliberately not guarded here: a general-purpose shell verb has no narrow contract to enforce, so that is its own decision. Flagging it explicitly so the boundary of this guard is not read as stronger than it is.

Not in scope

No ODB edits — Virgin.root is byte-identical. No UserTalk script changes. GUI/Mac build behavior unchanged.

[jsavin]

Gate Review: unit-891-openurl (PR #895)

Verdict: PASS WITH NOTES — fix the P1 pair before merge (house completionist rule: P2s too)

Tests (orchestrator-run merge record at 1e776a7)

• integration: PASS — 2491 total / 2313 passed / 0 failed / 161 skipped / 17 baselined, exit 0 (exact match with implementer's run)
• unit: two orchestrator runs both terminated on the known Latent: in-memory hash node takes on-disk resolution path -> NULL deref (dbgeteof via hashresolvevalue) #828 crasher (test_callback_infrastructure "Empty callback" SIGSEGV) and nothing else — failure set matches the known-crasher baseline; implementer's run at the same SHA was a clean exit 0
• new guard test: 15/15 (implementer-run; red pre-fix run demonstrated 7 failures incl. the literal Exec recorded: open http://127.0.0.1:5336/setupFrontier capture)

Reviewers

• bar-raiser ✓ ran (always) — needs changes: 2 P1, 3 P2
• security ✓ ran (always) — direction approved as strict narrowing: 2 P1, 1 P2
• concurrency ✓ ran (auto: frontier-cli/** path trigger) — no P0: 1 P1, 2 P2

All three converge: the only blocking-tier defects are error-path memory-ownership bugs in the new setpendingbrowserurl helper. The design, the exec-surface narrowing, the twin mirror, and the test harness were each independently verified sound.

---

P1 — fix before merge (deduplicated)

#	Finding	File	Flagged by	Fix
1	Double-dispose on setheapvalue failure: its contract (langvalue.c:596 comment) disposes the handle itself on failure; the helper then calls disposehandle(hvalue) on the freed handle. Trigger: tmp-stack enlarge failure under memory pressure → heap corruption in a long-running server.	tests/headless_sys_verbs.c:120-122	security, bar-raiser	Delete the disposehandle(hvalue) on that branch; just return (false).
2	Double-free/UAF on hashtableassign failure: val is still on the tmp stack; the branch calls disposehandle(hvalue) without removing the entry, so later tmp-stack cleanup frees it again — decoupled from the failing call, non-deterministic under thread-globals swapping. Deterministically reachable from pure script: pre-create system.temp.Frontier.pendingBrowserUrl as a TABLE (hashassign's protect block rejects non-external-over-external), then call sys.openUrl in default mode.	tests/headless_sys_verbs.c:125-128	concurrency, security, bar-raiser	Replace disposehandle(hvalue) with disposetmpvalue(&val) (lang.h:1175 — removes from tmp stack and disposes; canonical). Keep exemptfromtmpstack after the successful assign (matches the documented tmp-stack anti-pattern guidance), and add a one-line comment that the assign→exempt pair must remain yield-free (concurrency's fragility note).

Shared root cause: hand-rolled disposal conflicting with the setheapvalue/tmp-stack ownership contracts. ~4 lines total, one function.

P2 — fix in the same round (completionist)

#	Finding	File	Flagged by	Fix
3	allow_gui_browser declared outside #ifdef FRONTIER_HEADLESS is set-but-unused in a GUI compile — breaks a future -Werror GUI build.	Common/source/shellsysverbs.c (~:895)	bar-raiser	Scope the variable inside the ifdef; only use_agent_browser/fl_no_exec are needed outside.
4	Guard test runs read-write against the shared working DB (databases/Frontier.root); a dirty-flag propagation would make save_system_root_on_exit rewrite it every run → phantom md5 drift in staging checks.	tests/openurl_headless_guard_test.sh:24	bar-raiser	cp the DB into $STUBDIR and point --system-root at the copy (~3 lines).
5	log_info("…complete setup at %s") fires before flmarked is checked — log claims onboarding reached even when the marker write failed.	tests/headless_sys_verbs.c:806-818	security	Move the log after the flmarked check.
6	Twin comment claims "identical logic," but the classic TU never sets the marker — the observable contract diverges; next reader shouldn't trust "identical."	Common/source/shellsysverbs.c:855-859, 946-955	concurrency	One-line amendment stating the marker contract diverges (drift itself tracked in #893).

P2 — noted, no change required

• log_info on this path relies wholly on the GIL for serialization (framework-wide property; must never migrate to a GIL-released region) — concurrency.
• Theoretical settle-race in the guard script after unasserted exec runs; fix only if ever observed flaky (pattern-grep settle) — bar-raiser.

Clean dimensions (explicitly verified, not assumed)

• Exec surface strictly NARROWED: default headless returns before any fork in both TUs; allow-gui restores exactly pre-fix behavior; URL passes as a single argv element (no shell); mode parsing is fail-closed with length-guarded memcmp (no over-read of non-NUL handles, no prefix confusion).
• fork/exec child calls only async-signal-safe functions; default path no longer forks with the GIL held or blocks in waitpid; double-fork zombie reaping unchanged; no SIGCHLD handler exists to interfere.
• Marker: write is GIL-atomic (no yield points in findnamedtable/tablenewsubtable/hashtableassign chain — verified by trace); overwrite disposes prior value (no per-call leak); system.temp carries dontsave so the marker can never reach disk; no rehash hazard (fixed-bucket tables).
• Pascal length bytes verified arithmetically in BOTH TUs (76=\x4c); the pre-existing classic-file \x45 was a genuine 6-byte over-read, drive-by fix correct; PSTRING _Static_assert discipline is what caught it.
• Test harness: PATH stub scoped to the subprocess env, mktemp -d + trap cleanup, cannot false-PASS the no-exec assertions (late writes only flip PASS→FAIL); covers all 5 modes × (return, exec, marker) + empty-URL + unknown-value.
• All verb error branches unlock+dispose the URL handle (no leak); empty-URL rejection precedes mode logic (no marker on rejection, asserted by yaml).
• Style, Makefile wiring, yaml metadata, cleared stale skips: all conform.

Stats

• Files reviewed: 6 (+436/-22)
• Findings: 0 P0 · 2 P1 (deduplicated from 5 reviewer reports of the same root cause) · 4 actionable P2 · 2 noted
• Reviewers: 3 of 3 ran

Next step

Implementer fixes P1 #1-2 and P2 #3-6 in one round; re-run guard test + unit + integration at the new SHA; orchestrator re-runs the merge record and delta-reviews the changed function; then merge.