From 1c1493d1035770ec25f69fd688fbf6164f926b33 Mon Sep 17 00:00:00 2001 From: Raghu Betina Date: Wed, 23 Sep 2026 13:09:47 -0500 Subject: [PATCH] Skip attach refresh when its helpers are absent The planning archive is optional after root adoption. Guard both helper paths while preserving root precedence and real refresh errors. Keep existing container metadata and provider qualification separate. --- .devcontainer/devcontainer.json | 2 +- CONTRIBUTING.md | 15 ++++++--- README.md | 5 ++- script/check | 2 +- script/check-codespaces-private-port.mjs | 42 +++++++++++++++++++----- 5 files changed, 49 insertions(+), 17 deletions(-) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 5173657..7024001 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -34,5 +34,5 @@ "remoteUser": "vscode", "onCreateCommand": "sudo chown -R vscode:vscode /home/vscode/.claude /home/vscode/.codex /home/vscode/.cache", "postCreateCommand": ".devcontainer/setup-agents", - "postAttachCommand": "if [ -x script/refresh-codespaces-private-port ]; then exec script/refresh-codespaces-private-port; else exec .firstdraft/design/script/refresh-codespaces-private-port; fi" + "postAttachCommand": "if [ -x script/refresh-codespaces-private-port ]; then exec script/refresh-codespaces-private-port; elif [ -x .firstdraft/design/script/refresh-codespaces-private-port ]; then exec .firstdraft/design/script/refresh-codespaces-private-port; fi" } diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 892d251..cd75213 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -216,16 +216,21 @@ as the runtime discriminator and the current CLI's visibility command is the supported control surface. This is a containment for an observed provider registration defect, not a custom tunnel or application workaround. -The `postAttachCommand` resolves that same helper at `script/refresh-codespaces-private-port` before root adoption -and `.firstdraft/design/script/refresh-codespaces-private-port` afterward. This uses the standard +The `postAttachCommand` runs the executable helper at `script/refresh-codespaces-private-port` first, or at +`.firstdraft/design/script/refresh-codespaces-private-port` when only the archived helper remains. If neither is +executable, it succeeds without changing port registration. Retained planning context is optional for application +work; no helper is copied into generated application source to replace it. This uses the standard [Dev Container shell lifecycle](https://containers.dev/implementors/json_reference/#lifecycle-scripts), not a new service: the [reference implementation](https://github.com/devcontainers/cli/blob/main/src/spec-common/injectHeadless.ts) runs a string command in `/bin/sh` with the workspace as its working directory. An inline path selection survives the move even when the already-running container retains its original lifecycle configuration. Helper errors still propagate, including its active-listener refusal; no port policy changes with the path. The focused -`script/check-codespaces-private-port.mjs` exercises the configured command before and after a fixture's `script/` -directory moves under `.firstdraft/design/`, alongside the existing private-port and listener cases. That local proof -is not a new Codespaces observation. +`script/check-codespaces-private-port.mjs` exercises root precedence, archived execution, non-executable or removed +helpers, and the existing private-port and listener cases. Both helper locations preserve actual failures; the +guard does not turn a failed refresh into success. Existing containers can retain the lifecycle command recorded +when they were created; this source change does not rewrite their provider metadata. These shell checks do not +qualify fresh-template attachment or private preview after reattachment and stop/start. Those provider observations +remain under [Service #730](https://github.com/firstdraft/firstdraft/issues/730). The repaired tunnel exposed the already-recorded generated Rails HostAuthorization boundary. Do not copy the student template's broad `config.hosts.clear` or disabled origin check into Drawing Board. Generated-app host and diff --git a/README.md b/README.md index a242a14..84eacd5 100644 --- a/README.md +++ b/README.md @@ -439,7 +439,10 @@ run `chmod 600 .env` and try again. After root Compile, use the generated application's README and the exact Rails error instead of rerunning Drawing Board setup or its doctor. The old tooling is under `.firstdraft/design/`, and the existing container's PATH still reflects its pre-Compile layout. If a reconnect says the private-port refresh found an active listener, stop `bin/dev` before -rerunning `.firstdraft/design/script/refresh-codespaces-private-port`; do not weaken its listener guard. +rerunning `.firstdraft/design/script/refresh-codespaces-private-port` if you retained that helper; do not weaken its +listener guard. The current template skips this refresh when neither the original nor archived helper is executable, +so removing the optional planning archive does not require recreating its tools. An older Codespace can retain its +earlier attach command; see the [lifecycle details](CONTRIBUTING.md#work-on-the-template). If a Codespaces forwarded-port URL reaches Rails' **Blocked hosts** page, stop and tell your agent. Do not disable Rails host checks; the generated target must own that correction. diff --git a/script/check b/script/check index 06d7332..4f269b4 100755 --- a/script/check +++ b/script/check @@ -69,7 +69,7 @@ node script/check-image-receipt.mjs # shellcheck disable=SC1091 source .devcontainer/agent-versions.env -for script in script/agent-smoke .devcontainer/setup-agents bin/agent-doctor bin/review-plan-with-claude bin/review-plan-with-codex script/application-smoke script/check script/check-depth-one script/devcontainer-image-smoke script/devcontainer-smoke script/initialize-application script/selenium; do +for script in script/agent-smoke .devcontainer/setup-agents bin/agent-doctor bin/review-plan-with-claude bin/review-plan-with-codex script/application-smoke script/check script/check-depth-one script/devcontainer-image-smoke script/devcontainer-smoke script/initialize-application script/refresh-codespaces-private-port script/selenium; do bash -n "$script" if [[ ! -x "$script" ]]; then echo "$script must be executable." >&2 diff --git a/script/check-codespaces-private-port.mjs b/script/check-codespaces-private-port.mjs index a7f674b..a868ca5 100755 --- a/script/check-codespaces-private-port.mjs +++ b/script/check-codespaces-private-port.mjs @@ -284,8 +284,22 @@ esac assert.equal(fs.readFileSync(statePath, "utf8"), "private"); } - fs.mkdirSync(path.join(workspaceRoot, ".firstdraft", "design"), {recursive: true}); - fs.renameSync(path.join(workspaceRoot, "script"), path.join(workspaceRoot, ".firstdraft", "design", "script")); + const archiveRoot = path.join(workspaceRoot, ".firstdraft", "design"); + const archivedScriptRoot = path.join(archiveRoot, "script"); + const archivedRefresher = path.join(archivedScriptRoot, "refresh-codespaces-private-port"); + fs.mkdirSync(archivedScriptRoot, {recursive: true}); + fs.writeFileSync(archivedRefresher, "#!/bin/sh\nexit 99\n", {mode: 0o755}); + fs.writeFileSync(statePath, "private"); + fs.writeFileSync(logPath, ""); + const preferredRoot = run(); + assert.equal(preferredRoot.status, 0, preferredRoot.stderr); + assert.match(preferredRoot.stdout, /private visibility confirmed/); + + const rootFailure = run({MOCK_PUBLIC_FAILURE: "true"}); + assert.equal(rootFailure.status, 42, rootFailure.stderr); + + fs.rmSync(archivedScriptRoot, {recursive: true}); + fs.renameSync(path.join(workspaceRoot, "script"), archivedScriptRoot); for (let attach = 0; attach < 2; attach += 1) { fs.writeFileSync(statePath, "private"); fs.writeFileSync(logPath, ""); @@ -308,13 +322,23 @@ esac assert.equal(adoptedOutside.status, 0, adoptedOutside.stderr); assert.match(adoptedOutside.stdout, /skipped outside GitHub Codespaces/); - fs.renameSync(path.join(workspaceRoot, ".firstdraft", "design", "script"), path.join(workspaceRoot, ".firstdraft", "design", "missing-script")); - fs.writeFileSync(logPath, ""); - const missingHelper = run(); - assert.notEqual(missingHelper.status, 0); - assert.match(missingHelper.stderr, /\.firstdraft\/design\/script\/refresh-codespaces-private-port/); - assert.deepEqual(logLines(), []); - console.log("Codespaces post-attach contracts passed before and after root adoption, including listener guards."); + const archivedFailure = run({MOCK_PUBLIC_FAILURE: "true"}); + assert.equal(archivedFailure.status, 42, archivedFailure.stderr); + + fs.chmodSync(archivedRefresher, 0o644); + for (const archivePresent of [true, false]) { + if (!archivePresent) fs.rmSync(archiveRoot, {recursive: true}); + for (const listener of ["false", "true"]) { + fs.writeFileSync(logPath, ""); + const noExecutableHelper = run({MOCK_LISTENER: listener}); + assert.equal(noExecutableHelper.status, 0, noExecutableHelper.stderr); + assert.equal(noExecutableHelper.stdout, ""); + assert.equal(noExecutableHelper.stderr, ""); + assert.deepEqual(logLines(), []); + assert.equal(fs.readFileSync(statePath, "utf8"), "private"); + } + } + console.log("Codespaces post-attach contracts passed with root, archived, and absent helpers, preserving helper failures."); } finally { fs.rmSync(temporaryRoot, {recursive: true, force: true}); }