[Patriciaath999]

Close #564

The desktop app's child server never offered Change keys, because the code it asks for is printed to a terminal the child does not have.

• The app makes a new code at each launch and hands it over in CODETRIAL_CHANGE_KEYS_CODE. A code under 24 letters and digits leaves Change keys off.
• The app fills the code into the Setup page when that page loads in its window, so it stays out of URLs, history and the server log.
• A save prints codetrial: keys saved; restart to use them. On that line the app stops the child and launches it again, back on the lobby, without the failure page.

#564 asks which side should carry the code. This takes the shell-generates option, since it needs no parsing of the child's output. It is open to change if the other is preferred.

Known limits:

• With the interview in the default browser (no WebRTC in the web view), the app cannot fill the code there, so Change keys does not work in that mode, though the lobby still shows the link.
• An interview started in the moment between a save and the restart would be ended by the restart. The save's own refusal covers everything before that moment.

Verified on Linux (WSL): cargo test and cargo clippy --all-targets -- -D warnings in desktop/, and scripts/test.sh, where only review-memory-tests failed, because that machine's gh is too old for an attestation flag. The restart and the fill were not exercised in a running desktop window.

---

Summary by cubic

Closes #564: the desktop app's child server now offers Change keys, which it could not because the code it asks for is printed to a terminal the child does not have. The app makes a new code at each launch, hands it over in CODETRIAL_CHANGE_KEYS_CODE, and fills it into the Setup page when that page loads in its window, so it stays out of URLs, history, and the server log.

• A code under 24 letters and digits leaves Change keys off.
• A save prints codetrial: keys saved; restart to use them, which makes the app stop the child and launch it again, back on the lobby.
• Change keys does not work in default-browser mode, where the app cannot fill the code.
• An interview started between a save and the restart would be ended by the restart.

^{Written for commit 5137e37. Summary will update on new commits.}

[cubic-dev-ai]

4 issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/config.rs">

<violation number="1" location="src/config.rs:666">
P2: `change_keys_code` has no test in `tests/config.rs`, although its sibling helpers do. Add cases for a 23-character code (None), a 24-character alphanumeric code (Some), surrounding whitespace, and a code containing `-` or `_` (None), so the documented threshold and alphabet are pinned.</violation>
</file>

<file name="docs/desktop.md">

<violation number="1" location="docs/desktop.md:88">
P3: The intro still says the server starts with "five things" a launcher needs, but this PR adds a sixth, so the count now disagrees with the list and with "All six" below. Change "five" to "six" on the intro line.</violation>
</file>

<file name="desktop/src/server.rs">

<violation number="1" location="desktop/src/server.rs:24">
P2: This string is duplicated across the crate boundary and nothing pins the two copies together. If the server's text changes, the app never restarts after a save and the new keys stay unused, with no error. Add a CLI test that runs the real server and asserts it prints this exact line, as the CLI tests do for `BOUND_LINE`.</violation>
</file>

<file name="src/web/setup_edit.rs">

<violation number="1" location="src/web/setup_edit.rs:201">
P2: This restart signal is written before `save()` returns its success response, and the desktop closes stdin as soon as it reads the line. The child can exit before Hyper sends the response, leaving the form to report a failed save even though the config was written; emit the signal after response completion or defer the desktop stop until the request finishes.</violation>
</file>

<sub>Tip: Get a Slack DM when cubic finishes reviewing your PRs. Connect Slack

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: change_keys_code has no test in tests/config.rs, although its sibling helpers do. Add cases for a 23-character code (None), a 24-character alphanumeric code (Some), surrounding whitespace, and a code containing - or _ (None), so the documented threshold and alphabet are pinned.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/config.rs, line 666:

<comment>`change_keys_code` has no test in `tests/config.rs`, although its sibling helpers do. Add cases for a 23-character code (None), a 24-character alphanumeric code (Some), surrounding whitespace, and a code containing `-` or `_` (None), so the documented threshold and alphabet are pinned.</comment>

<file context>
@@ -660,6 +660,16 @@ pub fn exit_with_stdin(value: Option<&str>) -> bool {
+/// `CODETRIAL_CHANGE_KEYS_CODE`: the code the desktop app hands its child.
+/// One too short to stand in for the code `run_web` makes is no code, so a
+/// launcher that set it carelessly gets no Change keys rather than a weak one.
+pub fn change_keys_code(value: Option<&str>) -> Option<String> {
+    value
+        .map(str::trim)
</file context>

[cubic-dev-ai]

P2: This string is duplicated across the crate boundary and nothing pins the two copies together. If the server's text changes, the app never restarts after a save and the new keys stay unused, with no error. Add a CLI test that runs the real server and asserts it prints this exact line, as the CLI tests do for BOUND_LINE.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At desktop/src/server.rs, line 24:

<comment>This string is duplicated across the crate boundary and nothing pins the two copies together. If the server's text changes, the app never restarts after a save and the new keys stay unused, with no error. Add a CLI test that runs the real server and asserts it prints this exact line, as the CLI tests do for `BOUND_LINE`.</comment>

<file context>
@@ -19,6 +19,10 @@ use std::time::{Duration, Instant};
 
+/// What the server prints once Change keys has rewritten its config. The
+/// server's copy is `KEYS_SAVED_LINE` in src/web/setup_edit.rs.
+const KEYS_SAVED_LINE: &str = "codetrial: keys saved; restart to use them";
+
 /// How much of the server's stderr explains an exit. The reason is its last
</file context>

[cubic-dev-ai]

P2: This restart signal is written before save() returns its success response, and the desktop closes stdin as soon as it reads the line. The child can exit before Hyper sends the response, leaving the form to report a failed save even though the config was written; emit the signal after response completion or defer the desktop stop until the request finishes.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/web/setup_edit.rs, line 201:

<comment>This restart signal is written before `save()` returns its success response, and the desktop closes stdin as soon as it reads the line. The child can exit before Hyper sends the response, leaving the form to report a failed save even though the config was written; emit the signal after response completion or defer the desktop stop until the request finishes.</comment>

<file context>
@@ -192,6 +195,10 @@ impl SetupContext {
+
+        // The desktop app restarts its child on this line. Not `println!`,
+        // which panics on a stdout pipe nobody reads any more.
+        let _ = writeln!(std::io::stdout(), "{KEYS_SAVED_LINE}");
         json_response(StatusCode::OK, json!({ "saved": true }))
     }
</file context>

[cubic-dev-ai]

P3: The intro still says the server starts with "five things" a launcher needs, but this PR adds a sixth, so the count now disagrees with the list and with "All six" below. Change "five" to "six" on the intro line.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/desktop.md, line 88:

<comment>The intro still says the server starts with "five things" a launcher needs, but this PR adds a sixth, so the count now disagrees with the list and with "All six" below. Change "five" to "six" on the intro line.</comment>

<file context>
@@ -68,13 +68,24 @@ server's rules:
   CLI binary.
 
-All five are ordinary server options, and a CLI install can use them the same
+All six are ordinary server options, and a CLI install can use them the same
 way. `codetrial --help` lists the four flags, and the README's configuration
 table lists `CODETRIAL_ALLOWED_HOSTS`.
</file context>

[jserv]

The marker is written before json_response is returned, and the app answers it by closing the child's stdin, which makes the server process::exit(0) at once (exit_when_stdin_closes). The response to the Setup page's fetch('/api/setup') can be lost, so a save that worked shows "Request failed" and never shows "Saved." or the restart notice. Print the marker after the response has been written, or have restart in desktop/src/main.rs wait a moment before it stops the child.

[jserv]

server.stop(STOP_GRACE) ends the child at once, because stdin closing exits it, so the grace period does not apply. An interview admitted after the save's is_idle check and before this line is cut off with its report lost (the PR text acknowledges this). As a follow-up, once the save has written the config the server could refuse new interviews (restartPending is already tracked), so the window closes at the save rather than at the restart.

[jserv]

Please rebase this branch onto the latest main of sysprog21/codetrial before it is reviewed further. main now has the pre-PR review workflow: every commit carries a parent-linked Change-Id that the git hooks add, and the check job in CI verifies the chain. A branch that predates it has no Change-Ids, so that check cannot pass until the commits sit on top of the current main. Rebase and resolve any conflicts, run make hooks so the hooks are installed, then run python3 scripts/hashchain.py rechain to stamp the Change-Ids on the rebased commits and confirm the chain with python3 scripts/hashchain.py verify. A commit of 200 or more changed lines, or a pull request over 500 lines, also needs the interactive review described in docs/large-change-explanation.md before you push; a smaller change needs nothing extra.