Skip to content

fix: mode-aware directory creation + allow absolute bundle paths#10

Merged
danielmeppiel merged 6 commits intomainfrom
fix/ensure-working-dir-exists
Mar 11, 2026
Merged

fix: mode-aware directory creation + allow absolute bundle paths#10
danielmeppiel merged 6 commits intomainfrom
fix/ensure-working-dir-exists

Conversation

@danielmeppiel
Copy link
Copy Markdown
Collaborator

@danielmeppiel danielmeppiel commented Mar 11, 2026

Problem

Two bugs prevent gh-aw's apm-action integration from working end-to-end when using /tmp/ paths outside GITHUB_WORKSPACE:

Bug 1: ENOENT on isolated mode with non-existent working directory

In isolated mode with working-directory: /tmp/gh-aw/apm-workspace, the directory doesn't exist on the runner. generateManifest fails with:

Error: APM action failed: ENOENT: no such file or directory, open '/tmp/gh-aw/apm-workspace/apm.yml'

Workflow run: https://github.com/github/gh-aw/actions/runs/22928940243

Bug 2: Path traversal guard rejects absolute bundle paths in restore mode

The restore step uses bundle: /tmp/gh-aw/apm-bundle/*.tar.gz (downloaded by actions/download-artifact). resolveLocalBundle rejects this because the bundle is outside GITHUB_WORKSPACE:

Bundle path "/tmp/gh-aw/apm-bundle/*.tar.gz" resolves outside the workspace

This bug hadn't surfaced yet because the pack step (bug 1) failed first, skipping the agent job entirely.

Root Cause

Both bugs share the same root cause: the action assumed all paths are within GITHUB_WORKSPACE. gh-aw uses /tmp/gh-aw/ for isolation.

Fixes (5 commits)

Commit 1: Create working directory before generating manifest

Add fs.mkdirSync(resolvedDir, { recursive: true }) in run().

Commit 2: Allow absolute bundle paths

The path traversal check in resolveLocalBundle now only applies to relative patterns (where ../ traversal is the actual risk). Absolute paths are user-explicit and bypass the containment check.

Commit 3: Fix lint — use unknown instead of as any

The test mock parameter used as any with a misplaced eslint-disable comment. Changed parameter type to unknown for proper Jest mock compatibility — no cast or disable comment needed.

Commit 4: Make directory creation mode-aware with fail-fast

The unconditional mkdirSync from commit 1 was too broad — it silently created empty directories even in non-isolated mode, which would just fail later at apm install. Refined to a clear contract:

  • Isolated / pack / bundle modes (actionOwnsDir = true): the action creates the working directory. These modes bootstrap everything from scratch.
  • Non-isolated mode (actionOwnsDir = false): fail fast with a descriptive error if the directory doesn't exist, guiding the developer to use isolated: true.

Updated action.yml and README.md input descriptions to document this contract.

Commit 5: Use "non-isolated mode" terminology

Replaced "default mode" with "non-isolated mode" in all docs, comments, and error messages for clarity.

End-to-End Trace (gh-aw smoke workflow)

Pack step (activation job):

  1. resolvedDir = '/tmp/gh-aw/apm-workspace', actionOwnsDir = true (isolated)
  2. mkdirSync creates it ✅
  3. clearPrimitives — early return (no .github/) ✅
  4. generateManifest — writes apm.yml
  5. apm install + apm pack --target claude --archive
  6. Output: bundle-path: /tmp/gh-aw/apm-workspace/build/claude.tar.gz

Restore step (agent job):

  1. resolvedDir = GITHUB_WORKSPACE (no working-directory), actionOwnsDir = true (bundle)
  2. bundleInput = '/tmp/gh-aw/apm-bundle/*.tar.gz'
  3. resolveLocalBundle — absolute path, skips traversal check ✅
  4. extractBundle — extracts into GITHUB_WORKSPACE ✅

Test Coverage (22 tests, all pass)

  • run › creates working directory when it does not exist (isolated mode)
  • run › fails fast when working directory does not exist in non-isolated mode
  • resolveLocalBundle › allows absolute bundle paths outside workspace
  • resolveLocalBundle › throws when resolved path is outside workspace (relative traversal still blocked)

In isolated mode with a non-existent working directory (e.g.
/tmp/gh-aw/apm-workspace), generateManifest fails with ENOENT
because the directory doesn't exist yet.

Add fs.mkdirSync(resolvedDir, { recursive: true }) in run()
right after resolving the working directory, before any file
operations.

Also adds run() test coverage for isolated mode with a
non-existent directory, and expands mock setup to support
full run() testing.
Copilot AI review requested due to automatic review settings March 11, 2026 03:47
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes isolated-mode failures when working-directory points to a non-existent path by ensuring the directory exists before any file operations (e.g., generating apm.yml).

Changes:

  • Create the resolved working directory via fs.mkdirSync(..., { recursive: true }) early in run().
  • Add a run() integration-style test to verify non-existent nested working directories are created and apm.yml is written.
  • Update the compiled dist/index.js to reflect the source change.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/runner.ts Ensures the working directory exists before manifest generation / other filesystem work.
src/tests/runner.test.ts Adds coverage for isolated mode with a non-existent nested working directory.
dist/index.js Updates built action output to include the working-directory creation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +19 to +22
const mockExec = jest.fn<() => Promise<number>>();
jest.unstable_mockModule('@actions/exec', () => ({
exec: mockExec,
}));
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mockExec is typed as a no-arg function, but runApm calls @actions/exec.exec with (command, args, options). Consider typing the mock with the real signature so TypeScript can catch accidental misuse and so expectations like toHaveBeenCalledWith(...) are easier to write/maintain.

Copilot uses AI. Check for mistakes.
resolveLocalBundle rejected any bundle path outside the working
directory, including absolute paths like /tmp/gh-aw/apm-bundle/*.tar.gz
that the user explicitly specified. This blocks gh-aw's restore step
where download-artifact puts the bundle in /tmp/.

The path traversal check now only applies to relative patterns (where
traversal via ../ is the actual risk). Absolute paths are user-explicit
and bypass the containment check.

Added test: 'allows absolute bundle paths outside workspace'.
@danielmeppiel danielmeppiel changed the title fix: create working directory before generating manifest fix: create working directory + allow absolute bundle paths Mar 11, 2026
@danielmeppiel danielmeppiel requested a review from Copilot March 11, 2026 04:00
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…lt mode

- isolated/pack/bundle modes: action creates working directory (owns lifecycle)
- default mode: fail fast if directory doesn't exist (caller owns the project)
- Clear comments explaining the contract in runner.ts
- Updated action.yml and README.md input descriptions
- Added test: 'fails fast when working directory does not exist in default mode'
@danielmeppiel danielmeppiel requested a review from Copilot March 11, 2026 04:15
@danielmeppiel danielmeppiel changed the title fix: create working directory + allow absolute bundle paths fix: mode-aware directory creation + allow absolute bundle paths Mar 11, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/runner.ts
Move mutual-exclusion check above directory creation so invalid
configs never create directories as a side effect.
@danielmeppiel danielmeppiel merged commit 55646e0 into main Mar 11, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants