216 lines
10 KiB
Markdown
216 lines
10 KiB
Markdown
# Final Plan: fix interactive `cloud login` session teardown
|
|
|
|
## Goal
|
|
|
|
Fix the user-visible bug where interactive `cloud login` with a wrong password prints the retry
|
|
message, then drops back to a fresh CraftOS shell with lost in-memory command history instead of
|
|
prompting again.
|
|
|
|
One end-to-end fix, split into small independently testable changes. Before building all of it, we
|
|
**disambiguate the actual root cause** (see Step 0) so we know which fix is curing the bug and which
|
|
are hardening — the prior merged plan shipped three fixes across two competing theories without ever
|
|
confirming which one mattered.
|
|
|
|
## Confirmed code facts (verified against source)
|
|
|
|
These are grounded in the current tree, not hypotheses:
|
|
|
|
- `eventloop.lua:19` — `next_eventloop_id` is **module-local**. `eventloop.lua:174/186` —
|
|
`END_OF_LOOP` is keyed to that id, and `stopLoop()` stops a loop by queueing that global event.
|
|
Boot loop is created at `boot.lua:48`; the `cloud login` verifier loop at `cloud.lua:117`.
|
|
- `eventloop.lua:294` — handlers are dispatched **un-`pcall`'d**. The boot loop runs this under
|
|
`parallel.waitForAny(shellFn, eventLoopFn)` (`boot.lua:63,80`), so a throwing handler ends the
|
|
startup session and drops to a fresh shell.
|
|
- `libcloud.lua:256` — `connect()` calls `httpLike.websocketAsync(url)` **un-`pcall`'d**. The daemon
|
|
runs on `_G.bootEventLoop` (`servers/cloud.lua:45`), and `trapos_cloud_reconnect` →
|
|
`reconnectNow` → `connect()` (`libcloud.lua:415→425`). So a throw here escapes into the boot loop
|
|
and out through `waitForAny`.
|
|
|
|
## Two candidate root causes predict *different* symptoms
|
|
|
|
This is the key insight that drives Step 0.
|
|
|
|
- **A. Event-loop ID collision (clean exit).** Only possible if `require('/apis/eventloop')` hands
|
|
the cloud program a *separate module instance* from boot's. If so, both loops can take id `1`; the
|
|
verifier's `stopLoop()` queues `END_OF_LOOP/1`, the boot loop also matches it and exits **cleanly**
|
|
→ `waitForAny` returns → `startup` ends with **no error printed** → fresh shell. If `require` is a
|
|
singleton (normal CraftOS module cache), boot=id1 / verifier=id2+, names never collide, and this is
|
|
a non-cause.
|
|
- **B. Thrown handler (crash exit).** A throw in a boot-loop handler (e.g. websocket open) propagates
|
|
through `waitForAny`; bios prints a `startup:80:`-style **traceback** before the fresh shell.
|
|
|
|
The original report — "invalid password, then dropped to shell", no traceback mentioned — leans
|
|
toward **A (silent clean exit)**. Step 0 confirms which it is so the after-test is meaningful.
|
|
|
|
## Step 0: Disambiguate and reproduce the failure FIRST
|
|
|
|
On a disposable TrapOS computer (never the shared `8`/`10` machines — a wrong-password verify bounces
|
|
their live gateway connection):
|
|
|
|
1. Deploy the *current* (unfixed) `trapos-core` + `trapos-cloud`, set `cloud.url`, reboot so the
|
|
daemon runs.
|
|
2. `cloud login --force`, enter a **wrong** password.
|
|
3. Record exactly what happens:
|
|
- Was a Lua **traceback** printed before the shell returned? → cause is **B** (a throw); fix #3 is
|
|
load-bearing, fix #1 is unrelated hardening.
|
|
- Did it drop to the shell **silently** (only the "invalid password" line, then a fresh prompt)?
|
|
→ cause is **A** (collision); fix #1 is load-bearing.
|
|
4. Confirm the runtime instance question directly. Probe whether the cloud program actually gets a
|
|
separate eventloop module instance from boot:
|
|
|
|
```sh
|
|
# from a shell.run program context, compare against boot's loader identity / next id
|
|
just trapos-exec 'print(tostring(require("/apis/eventloop")))'
|
|
```
|
|
|
|
Compare the loader identity (and observed allocated ids) seen from boot vs. from a `shell.run`
|
|
program. If identical → singleton → collision (A) is impossible and the bug must be B.
|
|
|
|
Do not skip this. If the failure does not reproduce on demand it may be instance-identity dependent
|
|
and therefore intermittent; a post-fix "it re-prompts once" result would prove nothing.
|
|
|
|
## Bugs / Fix Areas
|
|
|
|
### 1. Event-loop stop can leak across loops
|
|
|
|
`packages/trapos-core/apis/eventloop.lua`.
|
|
|
|
**Primary fix — same-loop `stopRequested` (cleaner, more local).** The verifier's `finish()` always
|
|
calls `stopLoop()` from *inside its own handler*. So when `stopLoop()` is invoked while the loop is
|
|
actively dispatching its own handler/timeout, set an internal `stopRequested = true` and break after
|
|
the current dispatch — never queue a global event at all. This makes the verifier→boot leak
|
|
impossible regardless of ids.
|
|
|
|
**Belt-and-suspenders — process-global id counter.** Keep queueing the synthetic `END_OF_LOOP/<id>`
|
|
for *external* (cross-coroutine) stop calls, where the loop may be blocked in `os.pullEventRaw()` and
|
|
must be woken. To make those ids collision-proof across independently loaded module instances,
|
|
allocate the id from a `_G`-backed counter:
|
|
|
|
```lua
|
|
local COUNTER_KEY = '__trapos_next_eventloop_id';
|
|
local function nextEventLoopId()
|
|
local id = tonumber(_G[COUNTER_KEY]) or 1;
|
|
_G[COUNTER_KEY] = id + 1;
|
|
return id;
|
|
end
|
|
```
|
|
|
|
Preserve existing contracts: `api.STOP` unregister on successful handler return, duplicate-register
|
|
error, and "stopping an already stopped loop" error.
|
|
|
|
### 2. Boot event-loop handler errors can tear down the session
|
|
|
|
`packages/trapos-core/apis/eventloop.lua` + `packages/trapos-boot/startup/boot.lua`.
|
|
|
|
- Add protected handler dispatch as an explicit, opt-in event-loop option, e.g.
|
|
`createEventLoop({ onError = function(eventName, err) ... end })`.
|
|
- Use protected dispatch for `_G.bootEventLoop` in `boot.lua`; the error handler reports via
|
|
`printError` (else `print`) and the loop continues after a handler error.
|
|
- Keep dispatch **unprotected by default** for short-lived program-local loops so dev/test failures
|
|
still fail loudly unless explicitly isolated.
|
|
- Interaction to verify: after a handler throws under protected dispatch, the `api.STOP` unregister
|
|
(`:295`) and the `END_OF_LOOP`/`terminate` break (`:303`, which sits *outside* the handler loop)
|
|
must still run.
|
|
|
|
### 3. Cloud daemon websocket open can throw during reconnect
|
|
|
|
`packages/trapos-cloud/apis/libcloud.lua`.
|
|
|
|
- Forward-declare `scheduleReconnect` so `connect()` can call it.
|
|
- Wrap `httpLike.websocketAsync(url)` in `pcall`.
|
|
- On failure, log `websocket open failed` and call `scheduleReconnect()` instead of throwing.
|
|
- Avoid duplicate noisy logs while already in reconnect-loop mode.
|
|
|
|
## Out of scope (state the reasoning so it isn't re-litigated)
|
|
|
|
- **`error('Terminated', 0)` at `cloud.lua:182/205` (ctrl+t path).** Raised in the *cloud program*
|
|
(shellFn side), which `shell.run` already `pcall`s — it never reaches `waitForAny` and is not
|
|
fatal to the session. Protected boot dispatch (#2) does not and need not cover it. Leave as-is.
|
|
- **Stale hello `messageId` matching.** Useful cloud-login hardening, but it causes wrong-verdict
|
|
attribution, not session teardown. Follow-up only if Step 0 / manual testing still shows wrong
|
|
verdicts after the main fixes.
|
|
- **Persisted shell history.** Not a workaround for session teardown. Do not add.
|
|
|
|
## Implementation Steps
|
|
|
|
### Step 1: Regression coverage
|
|
|
|
`packages/trapos-core/tests/eventloop.lua`:
|
|
|
|
- **Same-loop stop:** a handler calls `stopLoop()` on its own loop; assert the loop stops after the
|
|
current dispatch and that no global `END_OF_LOOP` event is required (i.e. a *second* sibling loop
|
|
sharing the id is unaffected). This is the test that actually corresponds to the real failure
|
|
mode — frame it that way, not as the artificial double-`loadfile`.
|
|
- **Cross-instance id:** simulate independent module loads (`loadfile`), create a boot-like and a
|
|
verifier-like loop, stop the verifier from inside its handler, assert the boot-like loop still
|
|
handles a later probe event. (Belt-and-suspenders for the `_G` counter.) Explicitly stop the
|
|
boot-like loop at the end so the test cannot hang.
|
|
- **Protected dispatch:** one handler throws, a sibling/later handler still runs, and the configured
|
|
`onError` sink receives the error.
|
|
|
|
`packages/trapos-cloud/tests/cloud.lua`:
|
|
|
|
- `http.websocketAsync` throws during `startSession()`/reconnect; assert the error does not
|
|
propagate and a reconnect timer is scheduled.
|
|
|
|
Keep existing `cloud-program.lua` wrong-then-right interactive tests.
|
|
|
|
### Step 2: Fix `eventloop.lua`
|
|
|
|
- Add same-loop `stopRequested` break (primary).
|
|
- Replace module-local id allocation with the `_G` counter (secondary).
|
|
- Add opt-in protected dispatch (`onError`) for handlers (and timeout callbacks if practical).
|
|
- Preserve `api.STOP` unregister on success and the duplicate-register / already-stopped contracts.
|
|
|
|
### Step 3: Use protected boot event loop
|
|
|
|
`packages/trapos-boot/startup/boot.lua` — construct `_G.bootEventLoop` with an `onError` that reports
|
|
via `printError`/`print` and continues.
|
|
|
|
### Step 4: Harden cloud reconnect
|
|
|
|
`packages/trapos-cloud/apis/libcloud.lua` — forward-declare `scheduleReconnect`, `pcall` the
|
|
`websocketAsync` open, log + reschedule on failure, no duplicate noise while reconnecting.
|
|
|
|
### Step 5: Package versions
|
|
|
|
Bump owning packages and mirror in `packages/index.json`:
|
|
|
|
- `trapos-core` (`apis/eventloop.lua`)
|
|
- `trapos-boot` (`startup/boot.lua`)
|
|
- `trapos-cloud` (`apis/libcloud.lua`)
|
|
|
|
Do not bump the `trapos` meta-package unless release convention requires it.
|
|
|
|
## Verification
|
|
|
|
Targeted iteration:
|
|
|
|
```sh
|
|
just trapos-exec 'shell.run("/programs/runtest.lua", "--pretty", "/tests/eventloop.lua", "/tests/cloud.lua", "/tests/cloud-program.lua")'
|
|
```
|
|
|
|
Required after Lua/package edits:
|
|
|
|
```sh
|
|
just check
|
|
just test --pretty
|
|
```
|
|
|
|
Manual validation on the **same disposable computer used in Step 0** (so before/after is comparable):
|
|
|
|
1. Deploy updated `trapos-core`, `trapos-boot`, `trapos-cloud`; reboot.
|
|
2. `cloud login --force`, enter a wrong password → retry message prints **and the prompt returns**.
|
|
3. Cancel or complete login, then up-arrow → shell history still contains the command.
|
|
4. Enter the correct password → `logged in`.
|
|
5. `cloud status` → daemon recovered after the verify bounce.
|
|
|
|
Tie it back to Step 0: the symptom recorded there (silent vs. traceback) must be gone, and the fix
|
|
that addresses *that* cause must be the one demonstrated, not just "all three are in and it passes."
|
|
|
|
## Recommended split (three reviewable chunks)
|
|
|
|
1. Event-loop same-loop stop + cross-instance id safety.
|
|
2. Boot-loop protected dispatch.
|
|
3. Cloud reconnect crash-safety.
|