From 7f76d2c4aae2b34d914f5eaeff8751175ae892c0 Mon Sep 17 00:00:00 2001 From: Titouan Mathis Date: Tue, 8 Sep 2026 15:02:13 +0200 Subject: [PATCH 1/2] Create a sync destination's parent, and retry a lost create-script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs found by a real first deploy, neither covered by the tests. rsync makes the destination directory itself but never its parent, so a single file whose parent is missing fails with "No such file or directory". Whether a deploy worked therefore depended on the order of --sync: a project listing web/index.php before the web/wp that would have created web/ never got past the first file. The parent is now created by --rsync-path, which runs in the remote shell, rather than --mkpath, which needs rsync 3.2.3 at both ends and would fail on the option itself on an older runner. That failure exposed the second bug. The create-script was keyed on the project directory being absent, so the failed deploy — which had already cloned — left every later deploy treating it as an existing project and skipping the create-script. The environment stayed unseeded for good, with no recovery but to destroy it; here it left a project whose .env was never written and never would be. It is now keyed on a marker written only after the script succeeds, so a create-script that fails runs again next time. An environment created before this release carries no marker and is assumed to have been seeded: re-running a seed against a database that has been live since is worse than skipping a step that was probably already done. A .trafic-cloned marker separates that case from a first deploy of this version that failed before its create-script. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016xdQPh3UcnZfjM5HS36ABP --- CHANGELOG.md | 8 +++ packages/trafic-cli/src/commands/deploy.ts | 66 ++++++++++++++++++---- packages/trafic-cli/src/ssh.ts | 29 ++++++++++ packages/trafic-cli/test/deploy.test.ts | 53 +++++++++++++++++ packages/trafic-cli/test/ssh.test.ts | 33 +++++++++++ 5 files changed, 178 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a296571..391bd4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- **CLI**: `deploy` creates the destination's parent directory before a sync. rsync makes the destination directory itself but never its parent, so syncing a single file whose parent is missing failed with `No such file or directory` — and whether a deploy worked depended on the order of `--sync`: a project listing `web/index.php` before the `web/wp` that would have created `web/` never got past the first file. Found by a real first deploy, not by the tests. The parent is created by `--rsync-path`, which runs in the remote shell, rather than `--mkpath`, which needs rsync 3.2.3 at both ends and would fail on the option itself on an older runner ([#53]) +- **CLI**: `deploy` keys the create-script on a marker rather than on the project directory being absent. A first deploy that failed after the clone left the directory in place, so every later deploy took it for an existing project and skipped the create-script — the environment stayed unseeded for good, recoverable only by destroying it. Seen on the deploy that found the bug above: it left a project whose `.env` was never written and never would be. The marker is written only after the script succeeds, so a create-script that fails runs again next time. An environment created before this release carries no marker and is assumed to have been seeded, because re-running a seed against a live database is worse than skipping a step that was probably already done ([#53]) + ## [0.1.39] - 2026.09.08 ### Added @@ -498,6 +505,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#50]: https://github.com/studiometa/trafic/pull/50 [#51]: https://github.com/studiometa/trafic/pull/51 [#52]: https://github.com/studiometa/trafic/pull/52 +[#53]: https://github.com/studiometa/trafic/pull/53 [#31]: https://github.com/studiometa/trafic/pull/31 [GHSA-mw96-cpmx-2vgc]: https://github.com/advisories/GHSA-mw96-cpmx-2vgc [ddev/ddev#2696]: https://github.com/ddev/ddev/issues/2696 diff --git a/packages/trafic-cli/src/commands/deploy.ts b/packages/trafic-cli/src/commands/deploy.ts index 229c4b5..7c6e06f 100644 --- a/packages/trafic-cli/src/commands/deploy.ts +++ b/packages/trafic-cli/src/commands/deploy.ts @@ -67,6 +67,7 @@ export async function deploy( `cd ${projectDir}`, `mkdir -p .ddev`, `printf '${localConfig}' > .ddev/config.local.yaml`, + `touch ${CLONED_MARKER}`, ].join(" && "), ); } @@ -125,12 +126,7 @@ export async function deploy( // existing project: seeding is not idempotent — `ddev pull` overwrites the // database, which would discard the environment's content on every deploy. if (options.createScript) { - if (exists) { - info("Project already existed — skipping create-script"); - } else { - step("Run create-script"); - await io.exec(options, `cd ${projectDir} && ${options.createScript}`); - } + await runCreateScript(options, projectDir, exists, io); } // 6. Script inside DDEV container @@ -161,11 +157,16 @@ export async function deploy( const CONTAINER_SCRIPT = ".trafic-deploy.sh"; /** - * Quote a value for a shell single-quoted string. + * Written once this tool has cloned the repository. + * + * Its absence on a project that already has a `.git` means the environment + * was created by an earlier version, which left nothing on disk to say + * whether the create-script had run. */ -function shellQuote(value: string): string { - return `'${value.replace(/'/g, `'\\''`)}'`; -} +const CLONED_MARKER = ".trafic-cloned"; + +/** Written once the create-script has completed. */ +const CREATED_MARKER = ".trafic-created"; /** * Run the deploy script inside the DDEV container. @@ -189,7 +190,7 @@ async function runContainerScript( const script = [ "set -o errexit", - ...env.map(([key, value]) => `export ${key}=${shellQuote(value)}`), + ...env.map(([key, value]) => `export ${key}=${ssh.shellQuote(value)}`), options.script ?? "", "", ].join("\n"); @@ -211,6 +212,49 @@ async function runContainerScript( } } +/** + * Run the create-script, once per environment. + * + * Keyed on a marker rather than on the project directory being absent. A + * first deploy that fails after the clone — a sync error, say — leaves the + * directory in place, and keying on that made every later deploy skip the + * create-script: the environment stayed unseeded for good, with no way to + * recover but to destroy it. Seen on a real first deploy, which left a + * project whose `.env` was never written. + * + * The marker is written only after the script succeeds, so a create-script + * that fails runs again on the next deploy. + */ +async function runCreateScript( + options: DeployOptions, + projectDir: string, + existedBefore: boolean, + io: ssh.SshIo, +): Promise { + const created = await io.test(options, `test -f ${projectDir}/${CREATED_MARKER}`); + + if (created) { + info("Create-script already ran for this environment — skipping"); + return; + } + + const cloned = await io.test(options, `test -f ${projectDir}/${CLONED_MARKER}`); + + // An environment that predates the markers. Assume the create-script ran: + // re-running it would re-seed a database that has been live since, and + // losing that content is worse than skipping a step that was probably + // already done. Recorded so the question is settled from now on. + if (existedBefore && !cloned) { + info("Environment predates the create-script marker — assuming it ran"); + await io.exec(options, `touch ${projectDir}/${CREATED_MARKER}`); + return; + } + + step("Run create-script"); + await io.exec(options, `cd ${projectDir} && ${options.createScript}`); + await io.exec(options, `touch ${projectDir}/${CREATED_MARKER}`); +} + /** * Pin the project's router ports to the server's global DDEV setting. * diff --git a/packages/trafic-cli/src/ssh.ts b/packages/trafic-cli/src/ssh.ts index 7b71270..b7f01a5 100644 --- a/packages/trafic-cli/src/ssh.ts +++ b/packages/trafic-cli/src/ssh.ts @@ -209,6 +209,21 @@ export function formatDeletions( return `removed ${summary.files} ${plural} under: ${shown}${suffix}`; } +/** The parent of a remote path, for the `mkdir -p` that precedes a transfer. */ +export function parentOf(path: string): string { + const trimmed = path.replace(/\/+$/, ""); + const cut = trimmed.lastIndexOf("/"); + + // No slash, or only the leading one: the parent is the root or the cwd, + // both of which already exist + return cut > 0 ? trimmed.slice(0, cut) : cut === 0 ? "/" : "."; +} + +/** Quote a value for a single-quoted shell string. */ +export function shellQuote(value: string): string { + return `'${value.replace(/'/g, `'\\''`)}'`; +} + /** * Rsync a local path to the remote server. * @@ -249,6 +264,20 @@ export async function rsync( const args = [ "-azv", ...(isDirectory ? ["--delete"] : []), + // Create the destination's parent before the transfer starts. + // + // rsync makes the destination directory itself, but never its parent: a + // single file whose parent is missing fails with "No such file or + // directory", and so does a directory nested more than one level deep. + // Seen on a first deploy, where `web/index.php` came before the + // `web/wp` that would have created `web/` — the order of the sync list + // decided whether the deployment worked. + // + // `--rsync-path` rather than `--mkpath`: the flag needs rsync 3.2.3 on + // both ends, and a runner older than that would fail on the option + // itself. This runs in the remote shell, so any version does. + "--rsync-path", + `mkdir -p ${shellQuote(parentOf(remotePath))} && rsync`, "-e", sshCmd, isDirectory && !localPath.endsWith("/") ? `${localPath}/` : localPath, diff --git a/packages/trafic-cli/test/deploy.test.ts b/packages/trafic-cli/test/deploy.test.ts index 3fbb6ae..a833ce0 100644 --- a/packages/trafic-cli/test/deploy.test.ts +++ b/packages/trafic-cli/test/deploy.test.ts @@ -230,6 +230,59 @@ describe("deploy create-script", () => { deploy({ ...baseOptions, createScript: "ddev pull prod-db -y" }, io), ).rejects.toThrow(/ddev pull/); }); + + it("records that it ran, so the next deploy skips it", async () => { + const io = createFakeSshIo({ exists: false }); + + await deploy({ ...baseOptions, createScript: "ddev pull prod-db -y" }, io); + + const seed = io.commands.findIndex((c) => c.includes("ddev pull")); + const mark = io.commands.findIndex( + (c) => c.includes("touch") && c.includes(".trafic-created"), + ); + + expect(mark).toBeGreaterThan(seed); + }); + + it("does not record it when it fails", async () => { + const io = createFakeSshIo({ exists: false, fails: ["ddev pull"] }); + + await expect( + deploy({ ...baseOptions, createScript: "ddev pull prod-db -y" }, io), + ).rejects.toThrow(); + + // Recording a create-script that failed would strand the environment + // half-seeded: no later deploy would try again + expect(io.commands.some((c) => c.includes(".trafic-created"))).toBe(false); + }); + + it("runs again after a first deploy that failed before it", async () => { + // The clone succeeded and the sync failed, so the directory is there but + // the create-script never ran + const io = createFakeSshIo({ + tests: (command) => !command.includes(".trafic-created"), + }); + + await deploy({ ...baseOptions, createScript: "ddev pull prod-db -y" }, io); + + // Keying on the directory made this deploy skip the seed for good, + // leaving an environment that could only be destroyed and remade + expect(io.commands.some((c) => c.includes("ddev pull prod-db -y"))).toBe(true); + }); + + it("assumes an environment older than the markers was already created", async () => { + const io = createFakeSshIo({ + tests: (command) => + !command.includes(".trafic-cloned") && !command.includes(".trafic-created"), + }); + + await deploy({ ...baseOptions, createScript: "ddev pull prod-db -y" }, io); + + // Its database has been live for a while: re-seeding would discard that + expect(io.commands.some((c) => c.includes("ddev pull prod-db -y"))).toBe(false); + // Recorded, so the question is settled from now on + expect(io.commands.some((c) => c.includes(".trafic-created"))).toBe(true); + }); }); describe("deploy container script", () => { diff --git a/packages/trafic-cli/test/ssh.test.ts b/packages/trafic-cli/test/ssh.test.ts index 3ca6c1f..3b4cc7d 100644 --- a/packages/trafic-cli/test/ssh.test.ts +++ b/packages/trafic-cli/test/ssh.test.ts @@ -7,6 +7,7 @@ import { run, classifyPath, parseDuration, + parentOf, type CommandRunner, type ExecResult, } from "../src/ssh.js"; @@ -219,6 +220,19 @@ describe("rsync", () => { expect(calls[0]!.args).not.toContain("--delete"); }); + it("creates the destination's parent before transferring", async () => { + const { calls, runner } = recorder(); + + await rsync("web/index.php", "/x/web/index.php", defaultOptions, asFile, runner); + + // rsync makes the destination directory but never its parent: without + // this, syncing a file before the directory that would have created its + // parent failed with "No such file or directory" + const index = calls[0]!.args.indexOf("--rsync-path"); + expect(index).toBeGreaterThanOrEqual(0); + expect(calls[0]!.args[index + 1]).toBe("mkdir -p '/x/web' && rsync"); + }); + it("refuses a path that does not exist instead of reporting success", async () => { const { calls, runner } = recorder(); @@ -241,3 +255,22 @@ describe("classifyPath", () => { expect(classifyPath(join(src, "does-not-exist"))).toBe("missing"); }); }); + +describe("parentOf", () => { + it("drops the last segment", () => { + expect(parentOf("/home/ddev/www/app/web/index.php")).toBe( + "/home/ddev/www/app/web", + ); + expect(parentOf("~/www/app/vendor")).toBe("~/www/app"); + }); + + it("ignores a trailing slash", () => { + expect(parentOf("/x/y/")).toBe("/x"); + }); + + it("falls back to a path that always exists", () => { + // Nothing to create in either case, and "" would make mkdir fail + expect(parentOf("/top")).toBe("/"); + expect(parentOf("bare")).toBe("."); + }); +}); From 85b65e14c8dd6c68832cd14362f6163f9cf1276a Mon Sep 17 00:00:00 2001 From: Titouan Mathis Date: Tue, 8 Sep 2026 15:15:03 +0200 Subject: [PATCH 2/2] Keep the unit coverage floor out of the integration run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test:integration:ci` collects coverage for Codecov's `integration` flag, so it inherited the thresholds from vite.config.ts. Those are the unit suite's floor; the integration suite exercises ssh.ts against a real SSH server and touches little else, so it reports 66% and the run failed with all of its tests passing. CI has been red on main since 0.1.39 was released — the gate from #52 was measuring the wrong suite. Also adds the regression test for the parent-directory bug, against real rsync in the container. Verified to have teeth: with the fix removed it fails with the same "change_dir ... failed: No such file or directory" the deployment hit, and passes with it. 14 integration tests. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016xdQPh3UcnZfjM5HS36ABP --- CHANGELOG.md | 1 + packages/trafic-cli/test/integration/run.js | 13 ++++++++++- .../test/integration/ssh.integration.test.ts | 22 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 391bd4b..02f76e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **CLI**: `deploy` creates the destination's parent directory before a sync. rsync makes the destination directory itself but never its parent, so syncing a single file whose parent is missing failed with `No such file or directory` — and whether a deploy worked depended on the order of `--sync`: a project listing `web/index.php` before the `web/wp` that would have created `web/` never got past the first file. Found by a real first deploy, not by the tests. The parent is created by `--rsync-path`, which runs in the remote shell, rather than `--mkpath`, which needs rsync 3.2.3 at both ends and would fail on the option itself on an older runner ([#53]) - **CLI**: `deploy` keys the create-script on a marker rather than on the project directory being absent. A first deploy that failed after the clone left the directory in place, so every later deploy took it for an existing project and skipped the create-script — the environment stayed unseeded for good, recoverable only by destroying it. Seen on the deploy that found the bug above: it left a project whose `.env` was never written and never would be. The marker is written only after the script succeeds, so a create-script that fails runs again next time. An environment created before this release carries no marker and is assumed to have been seeded, because re-running a seed against a live database is worse than skipping a step that was probably already done ([#53]) +- **Dev**: The coverage floor no longer fails the integration run. `test:integration:ci` collects coverage for Codecov's `integration` flag, and so inherited the thresholds added in [#52] — which are the unit suite's floor, while the integration suite exercises `ssh.ts` against a real server and touches little else. The run failed at 66% with all of its tests passing, and CI has been red on `main` since 0.1.39 was released ([#53]) ## [0.1.39] - 2026.09.08 diff --git a/packages/trafic-cli/test/integration/run.js b/packages/trafic-cli/test/integration/run.js index fcf76db..6c3b241 100644 --- a/packages/trafic-cli/test/integration/run.js +++ b/packages/trafic-cli/test/integration/run.js @@ -148,7 +148,18 @@ function runTests(options = {}) { const args = ["vitest", "run", "test/integration/"]; if (withCoverage) { - args.push("--coverage"); + // Coverage is collected for Codecov's `integration` flag, but without + // the thresholds in vite.config.ts: those are the unit suite's floor, + // and this suite exercises ssh.ts against a real server while touching + // little else. Applying that floor here failed a run whose every test + // had passed — CI was red on main from 0.1.39 until this was fixed. + args.push( + "--coverage", + "--coverage.thresholds.statements=0", + "--coverage.thresholds.branches=0", + "--coverage.thresholds.functions=0", + "--coverage.thresholds.lines=0", + ); } // Set environment variables for tests diff --git a/packages/trafic-cli/test/integration/ssh.integration.test.ts b/packages/trafic-cli/test/integration/ssh.integration.test.ts index 687c418..2c6e010 100644 --- a/packages/trafic-cli/test/integration/ssh.integration.test.ts +++ b/packages/trafic-cli/test/integration/ssh.integration.test.ts @@ -126,6 +126,28 @@ describe("SSH Integration Tests", () => { } }); + it("syncs a single file whose remote parent does not exist", async () => { + const localDir = "/tmp/trafic-test-missing-parent"; + execSync(`mkdir -p ${localDir} && echo "scaffolded" > ${localDir}/index.php`); + + try { + // The shape that broke a real deploy: a file lands in `web/` before + // anything has created it. rsync makes the destination directory but + // never its parent, so this failed with "No such file or directory" + // and the deployment stopped at the first file of the sync list. + await rsync( + `${localDir}/index.php`, + `${testDir}/web/index.php`, + sshOptions, + ); + + const result = await exec(sshOptions, `cat ${testDir}/web/index.php`); + expect(result.stdout.trim()).toBe("scaffolded"); + } finally { + execSync(`rm -rf ${localDir}`); + } + }); + it("syncs a directory to remote", async () => { // Create a temp local directory with files const localDir = "/tmp/trafic-test-dir";