Seed initial build prompt for new apps (not existing-app sessions) - #96
Conversation
When a user creates a new app, the form description reached the build session only as the APP_DESCRIPTION env var, which the pod's CLAUDE.md merely *instructs* Claude to read — not guaranteed on turn one, so a carefully-described app could sit idle until the user re-typed it. Seed the description as Claude's opening prompt so a new app builds to spec immediately. Gate it strictly to brand-new apps: an existing-app "Build" session has no form, its description is back-filled from the app's stored sus.json (possibly stale), and the user is about to say what they want — auto-kicking a build there would risk clobbering intentional changes or nagging about outgrown specs. Detection uses the absence of sus.json (the existing scaffold gate), which is restart-safe: a recreated pod re-clones the committed scaffold so NEW_APP reads 0 and we don't re-kick on restart. The untrusted description is passed via an exported env var and expanded by the inner shell as a single quoted arg (never spliced into the command string), so it cannot break quoting or inject shell — verified with a hostile description containing quotes, $(...), backticks, and newlines. Also soften CLAUDE.md so resumed sessions treat the description as background rather than a spec to enforce. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @dev-dull's task in 4m 5s —— View job Review
The core idea is right, and the two things you called out in the description hold up:
One blocking issue and one adjacent bug, both posted inline: 1. The seed isn't one-shot ( 2. Minor, take or leave: No test coverage exists for |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if [ "${NEW_APP:-0}" = "1" ] && [ -n "${APP_DESCRIPTION:-}" ]; then | ||
| export SUS_INITIAL_PROMPT="Build this app now and show it in the preview pane on the right. Here is what it should do: | ||
|
|
||
| ${APP_DESCRIPTION} | ||
|
|
||
| Create the initial working version straight away, then tell me it's ready." | ||
| exec ttyd --port 8080 --writable --base-path / \ | ||
| bash -c "cd '$APP_DIR' && exec claude --dangerously-skip-permissions --model '${CLAUDE_MODEL:-opus}' \"\$SUS_INITIAL_PROMPT\"" | ||
| else | ||
| exec ttyd --port 8080 --writable --base-path / \ | ||
| bash -c "cd '$APP_DIR' && exec claude --dangerously-skip-permissions --model '${CLAUDE_MODEL:-opus}'" | ||
| fi |
There was a problem hiding this comment.
The seed isn't one-shot — it re-fires on every terminal reconnect inside the same pod, which can clobber a built app.
ttyd spawns a fresh instance of its command for each client connection, and NEW_APP is computed once at pod start and stays 1 for the pod's whole lifetime. So the restart-safety argument in the PR description holds for pod recreation (fresh entrypoint → scaffold already committed → NEW_APP=0), but not for a client reconnect against a still-live pod.
That's not hypothetical — the frontend reconnects on its own:
build.html:305-309— a not-ready blip clears →_loadTerminal()re-points the iframe at/terminal/?t=…→ new ttyd client → newclaude→ prompt re-fires.build.html:300-302— 3 consecutive not-ready polls (~15s: ttyd blip, proxy hiccup, pod-IP re-resolution) →window.location.reload()→ same thing.- Plus a manual refresh or a second tab.
build-pod/CLAUDE.mdalready documents that a page refresh loses the conversation, i.e. starts a newclaudeprocess in the same pod.
Pre-PR, a reconnect meant a fresh empty session (annoying, conversation lost, files untouched). Post-PR, that fresh session is handed "Build this app now … Create the initial working version straight away" against an app the user may have been iterating on for an hour — Claude will plausibly rewrite main.py/templates from the original (possibly already-superseded) description. The 5-minute autosave means the overwrite gets committed too.
Fix: move the decision into the inner shell and consume the seed atomically so only the first connection gets it.
| if [ "${NEW_APP:-0}" = "1" ] && [ -n "${APP_DESCRIPTION:-}" ]; then | |
| export SUS_INITIAL_PROMPT="Build this app now and show it in the preview pane on the right. Here is what it should do: | |
| ${APP_DESCRIPTION} | |
| Create the initial working version straight away, then tell me it's ready." | |
| exec ttyd --port 8080 --writable --base-path / \ | |
| bash -c "cd '$APP_DIR' && exec claude --dangerously-skip-permissions --model '${CLAUDE_MODEL:-opus}' \"\$SUS_INITIAL_PROMPT\"" | |
| else | |
| exec ttyd --port 8080 --writable --base-path / \ | |
| bash -c "cd '$APP_DIR' && exec claude --dangerously-skip-permissions --model '${CLAUDE_MODEL:-opus}'" | |
| fi | |
| if [ "${NEW_APP:-0}" = "1" ] && [ -n "${APP_DESCRIPTION:-}" ]; then | |
| SEED_FILE=/tmp/sus-initial-prompt | |
| cat > "$SEED_FILE" <<SEEDEOF | |
| Build this app now and show it in the preview pane on the right. Here is what it should do: | |
| ${APP_DESCRIPTION} | |
| Create the initial working version straight away, then tell me it's ready. | |
| SEEDEOF | |
| export SEED_FILE | |
| fi | |
| export APP_DIR | |
| export CLAUDE_MODEL="${CLAUDE_MODEL:-opus}" | |
| exec ttyd --port 8080 --writable --base-path / \ | |
| bash -c 'cd "$APP_DIR" || exit 1 | |
| if [ -n "${SEED_FILE:-}" ] && mv "$SEED_FILE" "$SEED_FILE.used" 2>/dev/null; then | |
| exec claude --dangerously-skip-permissions --model "$CLAUDE_MODEL" "$(cat "$SEED_FILE.used")" | |
| fi | |
| exec claude --dangerously-skip-permissions --model "$CLAUDE_MODEL"' |
The mv succeeds exactly once, so connection #2 falls through to the plain interactive session. Quoting safety is preserved: the unquoted <<SEEDEOF heredoc expands ${APP_DESCRIPTION} but bash doesn't re-scan the expansion, and the inner "$(cat …)" stays a single word — same guarantee your current \"\$SUS_INITIAL_PROMPT\" gives.
| NEW_APP=0 | ||
| if [ ! -f "$APP_DIR/sus.json" ]; then | ||
| NEW_APP=1 | ||
| cat > "$APP_DIR/sus.json" <<SUSJSON |
There was a problem hiding this comment.
Adjacent, pre-existing, but squarely in this PR's subject (routing APP_DESCRIPTION correctly) and easy to hit: this heredoc splices the raw description into JSON at line 83, so any " or newline in it produces an invalid sus.json.
The field is a <textarea> (landing/app/templates/new_app.html:92), so newlines are normal user behavior. And the failure is silent — landing/app/catalog.py:67-71 swallows json.JSONDecodeError and continues, so the app simply never appears in the catalog (and the git_workflow.py description back-fill reads the same manifest).
No injection risk — bash doesn't re-scan an expansion's contents — purely a validity bug. Since python3 is already used later in this script, generating the manifest with json.dump is the sturdy fix:
if [ ! -f "$APP_DIR/sus.json" ]; then
NEW_APP=1
APP_DIR="$APP_DIR" python3 - <<'PYEOF'
import json, os, datetime
data = {
"name": os.environ.get("APP_NAME") or "New App",
"description": os.environ.get("APP_DESCRIPTION", ""),
"owner": os.environ.get("USER_ID") or "anonymous",
"team": os.environ.get("APP_TEAM") or "_new",
"created_at": datetime.datetime.now(datetime.timezone.utc).strftime("%Y-%m-%d"),
"visibility": ["default"],
"default_stack": "python+htmx",
"tags": [],
}
with open(os.path.join(os.environ["APP_DIR"], "sus.json"), "w") as f:
json.dump(data, f, indent=2)
PYEOF(APP_NAME/APP_TEAM/APP_DESCRIPTION/USER_ID come from the pod env so the child already sees them; only APP_DIR needs passing since it's a local.) Perfectly fine to split this into its own PR if you'd rather keep this one tight.
Addresses the #96 review findings. 1. BLOCKING — seed was not one-shot. ttyd spawns a fresh `claude` per *client connection*, and the frontend reconnects on its own (terminal iframe reload on a not-ready blip, full page reload after ~15s, manual refresh, second tab). Baking the prompt into the ttyd command re-fired "build the initial version" on every reconnect — potentially over an app the user had been iterating on for an hour, with autosave committing the clobber. NEW_APP is fixed for the pod lifetime so it can't gate this. Fix: drop the prompt in a seed file and have the inner per-connection shell claim it with an atomic `mv` that succeeds exactly once; later connections fall through to a plain interactive session. Verified one-shot across sequential reconnects and exactly-once under 8 concurrent connections; injection safety preserved (hostile description reaches claude as a single "$(cat)" word). 2. Scaffold sus.json now uses python3 json.dump instead of a heredoc. APP_DESCRIPTION is a free-form <textarea>; a quote or newline spliced into hand-written JSON produced an invalid manifest, which catalog.py silently drops from the listing. Verified valid JSON with a description containing quotes, newlines and backslashes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @dev-dull's task in 3m 14s —— View job Review
Both findings from the last round are fixed correctly. 1. One-shot seed — 2. Quoting still holds across the rewrite. Two things I specifically checked:
One findingThe seed is burned by the first connection attempted, not the first that survives ( Minor
CoverageStill no test harness for |
| cd "$APP_DIR" || exit 1 | ||
| if [ -n "${SUS_SEED_FILE:-}" ] && mv "$SUS_SEED_FILE" "$SUS_SEED_FILE.used" 2>/dev/null; then | ||
| exec claude --dangerously-skip-permissions --model "$CLAUDE_MODEL" "$(cat "$SUS_SEED_FILE.used")" | ||
| fi |
There was a problem hiding this comment.
The seed is consumed by the first connection attempted, not the first one that survives — an early reconnect burns it and leaves a half-built app with a Claude that's been told to wait.
The mv is the right primitive for the clobber problem, and it fixes it. But the failure window it opens is the one this PR most cares about: the first ~60s of a brand-new app, while the seeded claude is mid-build.
_startHealthWatch (build.html:290-310) starts polling /status 5s after the terminal first loads. A single not-ready poll sets _reconnecting, and the moment it clears → _loadTerminal() → new iframe → new ttyd client. ttyd SIGHUPs the old client's process, so the building claude dies. The replacement connection finds the seed already renamed, falls through to the plain branch — and build-pod/CLAUDE.md now explicitly tells that session "don't start changing things unprompted, wait for the user." So the user lands on an empty prompt next to an app that got 30 seconds of scaffolding, with nothing indicating a build was ever started.
Pre-PR the same blip cost only the (empty) conversation. Post-PR it costs the build itself, silently.
A cheap re-arm keeps the one-shot guarantee where it matters (don't re-run a build over real work) while surviving a blip — only restore the seed if the app dir is still nothing but the scaffold:
| cd "$APP_DIR" || exit 1 | |
| if [ -n "${SUS_SEED_FILE:-}" ] && mv "$SUS_SEED_FILE" "$SUS_SEED_FILE.used" 2>/dev/null; then | |
| exec claude --dangerously-skip-permissions --model "$CLAUDE_MODEL" "$(cat "$SUS_SEED_FILE.used")" | |
| fi | |
| cd "$APP_DIR" || exit 1 | |
| # Re-arm a seed burned by a connection that died before Claude built | |
| # anything: safe precisely because "nothing but sus.json" means there is | |
| # no user work to clobber. | |
| if [ -f "${SUS_SEED_FILE:-}.used" ] && [ -z "$(ls -A . 2>/dev/null | grep -v '^sus\.json$')" ]; then | |
| mv "$SUS_SEED_FILE.used" "$SUS_SEED_FILE" 2>/dev/null || true | |
| fi | |
| if [ -n "${SUS_SEED_FILE:-}" ] && mv "$SUS_SEED_FILE" "$SUS_SEED_FILE.used" 2>/dev/null; then | |
| exec claude --dangerously-skip-permissions --model "$CLAUDE_MODEL" "$(cat "$SUS_SEED_FILE.used")" | |
| fi |
Once Claude has written even one file the re-arm stops firing, so the clobber protection you added is unchanged for every case where it matters. Entirely reasonable to skip this if you'd rather keep the tail simple — the current behaviour is still a net improvement over main, just worth knowing the seed isn't guaranteed to land.
Addresses the #96 re-review's remaining finding: the seed was consumed by the first connection *attempted*, not the first that survives. An early frontend reconnect (health-watch starts ~5s in) SIGHUPs the still- building claude before it writes anything; the replacement session found the seed already consumed and fell through to plain — leaving a half- scaffolded app and an idle prompt, in exactly the first-minute window this feature targets. Re-arm: before consuming the seed, if it was previously consumed AND the app dir still holds nothing but sus.json (no user work), restore it. Once Claude writes any file the re-arm stops firing, so the clobber protection is unchanged where it matters. Verified: fresh -> seeded, early reconnect before any file -> re-seeded, reconnect after a file exists -> plain. Also emit a trailing newline after json.dump so scaffold commits don't carry 'No newline at end of file'. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @dev-dull's task in 2m 58s —— View job Review
The re-arm addresses last round's finding, and the two mechanical fixes are correct:
One findingConcurrent connections can now double-seed ( Minor
CoverageUnchanged from last round, and now the tail has three interacting states ( |
| if [ -n "${SUS_SEED_FILE:-}" ] && [ -f "$SUS_SEED_FILE.used" ] \ | ||
| && [ -z "$(ls -A . 2>/dev/null | grep -v "^sus\.json$")" ]; then | ||
| mv "$SUS_SEED_FILE.used" "$SUS_SEED_FILE" 2>/dev/null || true | ||
| fi | ||
| if [ -n "${SUS_SEED_FILE:-}" ] && mv "$SUS_SEED_FILE" "$SUS_SEED_FILE.used" 2>/dev/null; then | ||
| exec claude --dangerously-skip-permissions --model "$CLAUDE_MODEL" "$(cat "$SUS_SEED_FILE.used")" | ||
| fi |
There was a problem hiding this comment.
The re-arm drops the mutual exclusion for concurrent connections, not just sequential ones — two clients in the pre-first-file window both get seeded and build the same empty dir at once.
Before this commit the mv was a true one-shot: at most one seeded claude could ever exist. Now the guard is "is the app dir still just sus.json", which says nothing about whether the previous claimant is still running. Two clients that overlap inside that window both re-arm and both claim:
- second browser tab / the build URL opened twice (ttyd serves each client its own process — no
--once/--max-clientshere); _loadTerminal()on a blip recovery (build.html:305-309) — the new ttyd client can connect before ttyd has reaped the old one'sclaude(SIGHUP delivery vs. a WebSocket handshake is a race, not an ordering).
Both processes then run claude --dangerously-skip-permissions with "Create the initial working version straight away" against the same directory: interleaved writes to main.py/requirements.txt, the auto-runner (entrypoint.sh:198-225) flapping as the detected stack appears twice, doubled token spend, and autosave committing whichever half-state wins. Sequential re-arm — the case you're fixing — is unaffected by adding a liveness check, because the whole point there is that the prior session is dead.
kill -0 is the cheap guard, and it needs no new package (pgrep/procps isn't in the image — build-pod/Dockerfile:7-12). $$ here is the pre-exec shell, so it stays valid as claude's PID:
| if [ -n "${SUS_SEED_FILE:-}" ] && [ -f "$SUS_SEED_FILE.used" ] \ | |
| && [ -z "$(ls -A . 2>/dev/null | grep -v "^sus\.json$")" ]; then | |
| mv "$SUS_SEED_FILE.used" "$SUS_SEED_FILE" 2>/dev/null || true | |
| fi | |
| if [ -n "${SUS_SEED_FILE:-}" ] && mv "$SUS_SEED_FILE" "$SUS_SEED_FILE.used" 2>/dev/null; then | |
| exec claude --dangerously-skip-permissions --model "$CLAUDE_MODEL" "$(cat "$SUS_SEED_FILE.used")" | |
| fi | |
| if [ -n "${SUS_SEED_FILE:-}" ] && [ -f "$SUS_SEED_FILE.used" ] \ | |
| && ! kill -0 "$(cat "$SUS_SEED_FILE.pid" 2>/dev/null)" 2>/dev/null \ | |
| && [ -z "$(ls -A . 2>/dev/null | grep -v "^sus\.json$")" ]; then | |
| mv "$SUS_SEED_FILE.used" "$SUS_SEED_FILE" 2>/dev/null || true | |
| fi | |
| if [ -n "${SUS_SEED_FILE:-}" ] && mv "$SUS_SEED_FILE" "$SUS_SEED_FILE.used" 2>/dev/null; then | |
| echo $$ > "$SUS_SEED_FILE.pid" | |
| exec claude --dangerously-skip-permissions --model "$CLAUDE_MODEL" "$(cat "$SUS_SEED_FILE.used")" | |
| fi |
A missing or malformed .pid makes kill -0 fail, so the re-arm still fires — first-reconnect-before-any-pid-file behaves as it does today.
…builds The re-arm added last commit asked only 'is the app dir still empty', not 'is the previous build dead'. So a second client arriving before the first build writes a file — a new tab, or a reconnect that beats SIGHUP to the old claude — would re-arm and launch a second --dangerously-skip-permissions build racing over the same directory. Record the claiming shell's pid (which becomes claude via exec) and only re-arm when that pid is no longer alive (kill -0; kill is a builtin, procps isn't in the image). This is orthogonal to the sequential early-death case the re-arm targets — there the prior session is dead by definition, so the guard passes and re-seeding still works. Verified: fresh -> seeded; second connection while the build is alive -> plain (no double build); after the builder dies with no files written -> re-seeded; after any file exists -> plain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @dev-dull's task in 2m 48s —— View job Review
One findingA missing Minor
Coverage
|
| if [ -n "${SUS_SEED_FILE:-}" ] && [ -f "$SUS_SEED_FILE.used" ] \ | ||
| && [ -z "$(ls -A . 2>/dev/null | grep -v "^sus\.json$")" ] \ | ||
| && ! { [ -f "$SUS_SEED_FILE.pid" ] && kill -0 "$(cat "$SUS_SEED_FILE.pid" 2>/dev/null)" 2>/dev/null; }; then | ||
| mv "$SUS_SEED_FILE.used" "$SUS_SEED_FILE" 2>/dev/null || true | ||
| fi |
There was a problem hiding this comment.
A missing .pid is treated as "previous claimant is dead", but it also means "a claim is in progress" — which leaves a (small) double-build window open.
The liveness check closes the wide race, but the claim isn't atomic with respect to the pid file. Line 332 renames the seed, and only line 335 publishes the pid — so between those two statements .used exists with no .pid:
A: mv seed .used ── window ── A: echo $$ > .pid ; exec claude (build 1)
B: [ -f .used ]=yes, [ -f .pid ]=no → kill -0 fails → re-arm → claim → build 2
Both then run --dangerously-skip-permissions over the same empty dir, which is the exact outcome this commit is meant to prevent. The window is sub-millisecond rather than seconds, so it's much narrower than what 77aac40 fixed — but the fix is a one-line reordering of the guard rather than a new file or lock.
Treat "no pid file yet" as in progress instead of dead: the claiming shell writes .pid as the very next statement after the mv with no I/O in between, so a .used with no .pid is effectively always a live claim, never an abandoned one.
| if [ -n "${SUS_SEED_FILE:-}" ] && [ -f "$SUS_SEED_FILE.used" ] \ | |
| && [ -z "$(ls -A . 2>/dev/null | grep -v "^sus\.json$")" ] \ | |
| && ! { [ -f "$SUS_SEED_FILE.pid" ] && kill -0 "$(cat "$SUS_SEED_FILE.pid" 2>/dev/null)" 2>/dev/null; }; then | |
| mv "$SUS_SEED_FILE.used" "$SUS_SEED_FILE" 2>/dev/null || true | |
| fi | |
| if [ -n "${SUS_SEED_FILE:-}" ] && [ -f "$SUS_SEED_FILE.used" ] \ | |
| && [ -f "$SUS_SEED_FILE.pid" ] \ | |
| && ! kill -0 "$(cat "$SUS_SEED_FILE.pid" 2>/dev/null)" 2>/dev/null \ | |
| && [ -z "$(ls -A . 2>/dev/null | grep -v "^sus\.json$")" ]; then | |
| mv "$SUS_SEED_FILE.used" "$SUS_SEED_FILE" 2>/dev/null || true | |
| fi |
Behaviour is otherwise unchanged: an empty or malformed .pid still makes kill -0 fail, so the sequential re-arm you added keeps working. The loser of the race falls through to a plain interactive session, which is the pre-existing (and correct) outcome for a second concurrent client.
…sence The claim mv and the pid write are not atomic, so there is a sub-ms window where $SUS_SEED_FILE.used exists but the .pid is not written yet. The prior guard treated a missing .pid as 'claimant dead' and would re-arm there, re-opening the concurrent double-build this was meant to close. Require .pid to exist AND its pid to be dead before re-arming; a missing .pid now means 'claim in flight' (live claimant) and is left alone. A builder that dies in that sub-ms gap degrades to a plain session, never a double build. Verified by running the actual shipped bash -c body through the matrix: TOCTOU window -> plain, fresh -> seeded, second connection while alive -> plain, dead-with-no-files -> re-seeded, post-work -> plain, existing app -> plain. (Also caught + fixed an apostrophe in the inner comment that would have terminated the single-quoted bash -c string — bash -n now clean.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @dev-dull's task in 2m 51s —— View job Review
One findingThe stale If the round-per-race pattern is wearing thin, an atomic CoverageUnchanged, and now the most load-bearing part of this PR is a four-state protocol ( |
| if [ -n "${SUS_SEED_FILE:-}" ] && [ -f "$SUS_SEED_FILE.used" ] \ | ||
| && [ -f "$SUS_SEED_FILE.pid" ] \ | ||
| && ! kill -0 "$(cat "$SUS_SEED_FILE.pid" 2>/dev/null)" 2>/dev/null \ | ||
| && [ -z "$(ls -A . 2>/dev/null | grep -v "^sus\.json$")" ]; then | ||
| mv "$SUS_SEED_FILE.used" "$SUS_SEED_FILE" 2>/dev/null || true | ||
| fi |
There was a problem hiding this comment.
[ -f .pid ] only makes .pid-absence mean "claim in flight" for the first claim — after any re-arm a stale .pid survives and reopens the same window.
b61633e is correct about the invariant it wants (.used with no .pid = live claimant), but the re-arm path never clears .pid, so the file outlives the claimant that wrote it. Walk two re-arms:
C1: mv seed .used ; echo $$ > .pid ; exec claude C1 dies (reconnect SIGHUP)
C2: guard ✓ → mv .used seed → mv seed .used ── window ── C2: echo $$ > .pid
C3: [ -f .used ]=yes
[ -f .pid ]=yes ← stale, still C1's pid
kill -0 C1 = fails → "dead"
dir = just sus.json = yes
→ re-arm → claim → build 2, concurrent with C2's
The guard reads C1's corpse as evidence that C2 is dead. Same sub-millisecond window and same outcome as the one this commit closes — two --dangerously-skip-permissions builds interleaving over one empty dir — just now reachable only on the second and later re-arms rather than the first. That's not an obscure path: the re-arm exists for reconnect blips, and blips cluster, so "a prior claimant already died" is the normal state by the time a race matters.
One line fixes it — drop the stale pid as part of re-arming, so absence consistently means in-flight:
| if [ -n "${SUS_SEED_FILE:-}" ] && [ -f "$SUS_SEED_FILE.used" ] \ | |
| && [ -f "$SUS_SEED_FILE.pid" ] \ | |
| && ! kill -0 "$(cat "$SUS_SEED_FILE.pid" 2>/dev/null)" 2>/dev/null \ | |
| && [ -z "$(ls -A . 2>/dev/null | grep -v "^sus\.json$")" ]; then | |
| mv "$SUS_SEED_FILE.used" "$SUS_SEED_FILE" 2>/dev/null || true | |
| fi | |
| if [ -n "${SUS_SEED_FILE:-}" ] && [ -f "$SUS_SEED_FILE.used" ] \ | |
| && [ -f "$SUS_SEED_FILE.pid" ] \ | |
| && ! kill -0 "$(cat "$SUS_SEED_FILE.pid" 2>/dev/null)" 2>/dev/null \ | |
| && [ -z "$(ls -A . 2>/dev/null | grep -v "^sus\.json$")" ]; then | |
| # Retire the dead claimant's pid *before* re-arming: a .pid left on | |
| # disk would let the next connection read this corpse as proof that | |
| # the incoming claimant is dead and re-arm underneath it. | |
| rm -f "$SUS_SEED_FILE.pid" | |
| mv "$SUS_SEED_FILE.used" "$SUS_SEED_FILE" 2>/dev/null || true | |
| fi |
Every interleaving after that resolves to exactly one seeded session — a C3 landing between the rm and the mv sees no .pid and doesn't re-arm, then loses the claim mv because the seed isn't back yet; one landing after the mv skips the re-arm on -f .used and races only on the claim mv, which is a real mutex.
If you'd rather stop playing whack-a-mole with this two-file protocol, the structural version is an atomic mkdir "$SUS_SEED_FILE.lock" around claim-plus-pid-write — one primitive instead of an ordering argument. Either is fine; the rm -f is the smaller diff and I don't see a hole in it.
The re-arm (restore the seed if an early reconnect killed the build before it wrote a file) turned a one-line convenience into a four-state protocol with a liveness check, and every review round surfaced another race in it (pid TOCTOU, stale pid, ...). It's not worth it for a first-turn nicety. Keep only the part that matters: consume the seed with an atomic mv that succeeds exactly once, so the first connection is seeded and reconnects never re-run the build over the user's work. If an early blip kills the first build before any file is written, the replacement session is plain and the user simply restates the request — non-destructive, and still strictly better than main (which never seeds at all). Removing the re-arm deletes the entire concurrent-double-build race class the last three rounds chased. Verified against the shipped bash -c body: fresh -> seeded, reconnect -> plain, 8 concurrent -> exactly one seeded, existing app -> plain, hostile description stays a single argv word. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @dev-dull's task in 9m 53s —— View job Review
No findings this round. What I re-checked against the simplified tail:
Minor
CoverageStill no harness for Looks good to merge after the smoke test you already flagged. |
1 participant
Problem
When a user creates a new app, the form description reaches the build session only as the
APP_DESCRIPTIONenvironment variable. Nothing feeds it into Claude's actual conversation — the pod'sCLAUDE.mdmerely instructs Claude to go read that env var. That's a soft nudge, not a guarantee: on turn one Claude's context holds the instruction but not the value, so a carefully-described app can sit idle until the user re-types what they wanted.Fix
Seed the description as Claude's opening prompt so a new app builds to spec immediately.
Critically, this is gated to brand-new apps only. An existing-app "Build" session is different:
sus.json(git_workflow.py:120-129), which may be stale;Auto-kicking a build there would risk clobbering intentional changes or nagging about a spec the app has outgrown. So existing-app sessions stay plain interactive, unchanged.
New-vs-existing detection
Reuses the existing scaffold gate: absence of
sus.json= brand-new app (an existing app's clone brings its ownsus.jsondown). This is restart-safe — a recreated pod re-clones the now-committed scaffold, soNEW_APPreads 0 and we don't re-kick a build when a pod restarts mid-session.Safety
APP_DESCRIPTIONis untrusted user input. It's passed via an exported env var and expanded by the inner shell as a single quoted argument (escaped\$SUS_INITIAL_PROMPT) — never spliced into the command string. Verified with a hostile description containing",$(...), backticks,;, and newlines:claudereceives it as exactly one literal argument and nothing is evaluated as shell (no/tmp/PWNED).Also
Softens
build-pod/CLAUDE.mdso a resumed session treats the description as background about the app's original intent rather than a spec to enforce — don't assume divergence is a bug, wait for the user's direction.Verification
bash -nclean;shellcheckclean.Not yet exercised live in a pod — worth a smoke test on a cluster before/after merge (create a new app with a description → confirm Claude starts building immediately; click Build on an existing app → confirm it waits).
🤖 Generated with Claude Code