Skip to content

fix(plugin-runtime): write desktop command output to the OS temp dir - #530

Open
Agnik47 wants to merge 1 commit into
agentrhq:mainfrom
Agnik47:fix/plugin-runtime-os-tmpdir
Open

Agnik47 wants to merge 1 commit into
agentrhq:mainfrom
Agnik47:fix/plugin-runtime-os-tmpdir

Conversation

@Agnik47

@Agnik47 Agnik47 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Problem

makeScreenshotCommand and makeDumpCommand in src/plugin-runtime.ts — both
part of the public @agentrhq/webcmd/plugin-runtime surface — hardcode
/tmp/<site>-* as their output destination.

/tmp is not a temp directory on Windows. Node resolves it against the current
drive root:

path.resolve('/tmp/x-dom.html')  // → D:\tmp\x-dom.html
os.tmpdir()                      // → C:\Users\<user>\AppData\Local\Temp

So on Windows <site> screenshot and <site> dump silently mkdirSync a stray
\tmp folder at the root of whichever drive the process happens to run from, and
fail with EPERM when that root is not writable for the current user (the common
case for a non-elevated user on C:\).

The repo supports Windows deliberately — win32 branches already exist in
plugin.ts, discovery.ts, skills.ts, external.ts and errors.ts — so these
two defaults are the odd ones out.

Fix

Use os.tmpdir() for both defaults and for the --output help text. Five lines in
src/plugin-runtime.ts. Behaviour is unchanged on Linux (os.tmpdir() is /tmp),
and an explicit output argument is still honoured exactly as before.

Tests

  • The existing desktop command factories test was asserting the /tmp literals
    itself, so it is rewritten against os.tmpdir().
  • New test defaults screenshot and dump output to the OS temp dir: relocates the
    temp directory through TMPDIR (POSIX) / TEMP+TMP (Windows) — os.tmpdir()
    reads those on every call, so no builtin-module mocking is needed — then asserts
    the help text, both returned paths, and every file actually written land inside
    it. This fails on Linux too, not just Windows.

Verification

  • npx vitest run --project unit src/plugin-runtime.test.ts → 15 passed.
  • Stashed the src/plugin-runtime.ts change and re-ran the same file → 2 failed,
    reporting /tmp/runtime-test-… against the expected temp dir, confirming the new
    test genuinely catches the regression rather than passing vacuously.
  • npx tsc --noEmit → clean.
  • Grepped docs/, skills/, skill-src/, mcp-skills/ and plugins/ for these
    paths — nothing documents them, so no docs change is needed.

Deliberately out of scope

src/browser/command-catalog.ts:25 also mentions /tmp/upload.pdf, but that is a
help-text example of a caller-supplied path, not a destination webcmd writes to.

@github-actions

github-actions Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

🟠 Maintainer review suggested — low confidence

The automated review could not reach a fully supported conclusion.

Limitations

  • Some automated findings could not be verified against the pull request diff.

This review is advisory and does not block merging.

`makeScreenshotCommand` and `makeDumpCommand` hardcoded `/tmp/<site>-*` as
their output destination. `/tmp` is not a temp directory on Windows: Node
resolves it against the current drive root, so `<site> screenshot` and
`<site> dump` create a stray `\tmp` folder at the root of whichever drive
the process runs from, and fail outright with EPERM when that root is not
writable for the current user.

Use `os.tmpdir()` for both defaults and for the `--output` help text, so the
shipped `@agentrhq/webcmd/plugin-runtime` factories behave the same on every
platform. An explicit `output` argument is still honoured unchanged.
@Agnik47
Agnik47 force-pushed the fix/plugin-runtime-os-tmpdir branch from 72004fe to 527917f Compare October 2, 2026 04:22
@Agnik47

Agnik47 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main. The branch had the mascot commit from #511 stacked underneath it by mistake, which was the source of the conflict; I dropped it, so this PR is now only the two plugin-runtime files.

This branch has not been deployed

No deployments
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