From 33e8cf4889839a3da41fdb3a4acb346329e3341d Mon Sep 17 00:00:00 2001 From: Emilien Escalle Date: Mon, 13 Apr 2026 21:31:59 +0200 Subject: [PATCH] feat(local-actions): enhance destination path handling and cleanup logic Signed-off-by: Emilien Escalle --- actions/local-actions/LocalActionsManager.js | 24 ++++++++++++++++--- .../local-actions/LocalActionsManager.test.js | 15 +++++++++++- actions/local-actions/cleanup.js | 3 ++- actions/local-actions/index.test.js | 16 ++++++++++--- 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/actions/local-actions/LocalActionsManager.js b/actions/local-actions/LocalActionsManager.js index 9fc5e34..056b1c3 100644 --- a/actions/local-actions/LocalActionsManager.js +++ b/actions/local-actions/LocalActionsManager.js @@ -15,7 +15,11 @@ export class LocalActionsManager { } await mkdir(path.dirname(destinationPath), { recursive: true }); - await symlink(sourceDirectory, destinationPath, this.#getSymlinkType()); + const symlinkTarget = this.#resolveSymlinkTarget( + sourceDirectory, + destinationPath, + ); + await symlink(symlinkTarget, destinationPath, this.#getSymlinkType()); return { created: true, @@ -29,6 +33,7 @@ export class LocalActionsManager { } await rm(destinationPath, { force: true, recursive: true }); + return true; } @@ -37,8 +42,7 @@ export class LocalActionsManager { throw new Error("Workspace path is required."); } - const normalizedWorkspacePath = path.resolve(workspacePath); - return path.resolve(normalizedWorkspacePath, "../self-actions"); + return path.resolve(workspacePath, "../self-actions"); } async resolveSourceDirectory({ sourcePath }) { @@ -74,4 +78,18 @@ export class LocalActionsManager { #getSymlinkType() { return process.platform === "win32" ? "junction" : "dir"; } + + /** + * Computes the symlink target. Uses a relative path on non-Windows + * platforms so the symlink resolves correctly from both the container + * filesystem and the host filesystem (bind-mount path differs). + * Windows junctions require absolute targets. + */ + #resolveSymlinkTarget(sourceDirectory, destinationPath) { + if (this.#getSymlinkType() === "junction") { + return sourceDirectory; + } + + return path.relative(path.dirname(destinationPath), sourceDirectory); + } } diff --git a/actions/local-actions/LocalActionsManager.test.js b/actions/local-actions/LocalActionsManager.test.js index 467985e..28c0a6d 100644 --- a/actions/local-actions/LocalActionsManager.test.js +++ b/actions/local-actions/LocalActionsManager.test.js @@ -4,6 +4,7 @@ import { mkdtempSync, mkdirSync, readFileSync, + readlinkSync, realpathSync, rmSync, writeFileSync, @@ -53,7 +54,7 @@ const createFixture = () => { }; }; -test("prepare creates a symlink to sibling actions in the destination", async () => { +test("prepare creates a relative symlink to sibling actions in the destination", async () => { const fixture = createFixture(); const manager = new LocalActionsManager(); @@ -66,6 +67,16 @@ test("prepare creates a symlink to sibling actions in the destination", async () assert.equal(result.created, true); assert.equal(result.destinationPath, fixture.selfActionsPath); assert.equal(lstatSync(fixture.selfActionsPath).isSymbolicLink(), true); + + // Verify the symlink uses a relative target + const linkTarget = readlinkSync(fixture.selfActionsPath); + assert.equal( + path.isAbsolute(linkTarget), + false, + `Expected relative symlink target, got: ${linkTarget}`, + ); + + // Verify it resolves to the correct directory assert.equal( realpathSync(fixture.selfActionsPath), fixture.actionsDirectory, @@ -141,6 +152,8 @@ test("cleanup removes the destination only when it was created by the action", a }), true, ); + assert.equal(existsSync(fixture.selfActionsPath), false); + assert.equal( await manager.cleanup({ created: false, diff --git a/actions/local-actions/cleanup.js b/actions/local-actions/cleanup.js index 472ecde..f511419 100644 --- a/actions/local-actions/cleanup.js +++ b/actions/local-actions/cleanup.js @@ -6,8 +6,9 @@ const manager = new LocalActionsManager(); try { const destinationPath = runtime.getState("local_actions_destination_path"); + const created = runtime.getState("local_actions_created") === "true"; const cleaned = await manager.cleanup({ - created: runtime.getState("local_actions_created") === "true", + created, destinationPath, }); diff --git a/actions/local-actions/index.test.js b/actions/local-actions/index.test.js index 3220ad9..5bbddd0 100644 --- a/actions/local-actions/index.test.js +++ b/actions/local-actions/index.test.js @@ -4,6 +4,7 @@ import { mkdtempSync, mkdirSync, readFileSync, + readlinkSync, realpathSync, rmSync, writeFileSync, @@ -70,7 +71,7 @@ const runNodeScript = (scriptPath, env) => }, }); -test("index.js writes outputs and creates the local actions symlink", () => { +test("index.js writes outputs, creates a relative symlink, and saves state", () => { const fixture = createFixture(); try { @@ -95,6 +96,15 @@ test("index.js writes outputs and creates the local actions symlink", () => { ), ); assert.equal(lstatSync(fixture.selfActionsPath).isSymbolicLink(), true); + + // Verify relative symlink + const linkTarget = readlinkSync(fixture.selfActionsPath); + assert.equal( + path.isAbsolute(linkTarget), + false, + `Expected relative symlink target, got: ${linkTarget}`, + ); + assert.equal( realpathSync(fixture.selfActionsPath), fixture.actionsDirectory, @@ -111,11 +121,11 @@ test("index.js writes outputs and creates the local actions symlink", () => { ); assert.match( readFileSync(fixture.stateFile, "utf8"), - /^local_actions_created<<.+\ntrue\n.+\n/s, + /local_actions_created<<.+\ntrue\n.+\n/s, ); assert.match( readFileSync(fixture.stateFile, "utf8"), - /^local_actions_created<<.+\ntrue\n.+\nlocal_actions_destination_path<<.+\n.*self-actions\n.+\n$/s, + /local_actions_destination_path<<.+\n.*self-actions\n.+\n/s, ); } finally { fixture.teardown();