Skip to content

Fix agents version stderr parsing#547

Merged
ralyodio merged 4 commits into
profullstack:masterfrom
ayskobtw-lil:codex/agents-version-stderr
Jun 2, 2026
Merged

Fix agents version stderr parsing#547
ralyodio merged 4 commits into
profullstack:masterfrom
ayskobtw-lil:codex/agents-version-stderr

Conversation

@ayskobtw-lil
Copy link
Copy Markdown
Contributor

Fixes #546.

What changed

  • agents list now falls back to stderr when stdout is empty for successful <binary> --version probes.
  • Added regression coverage for an installed agent whose version command exits 0 and writes claude 1.2.3 to stderr.

Why

Some CLIs print version information on stderr. The previous result.stdout ?? result.stderr expression ignored stderr whenever stdout was an empty string, so JSON output reported installed agents with version: "".

Verification

  • npx --yes pnpm@9.12.0 exec vitest run packages/cli/src/commands/agents.test.ts -> 4 passed
  • npx --yes pnpm@9.12.0 --filter @profullstack/sh1pt typecheck -> passed

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 1, 2026

Greptile Summary

This PR fixes a stdout/stderr fallback bug in probeAgent where an empty stdout string silently suppressed the stderr fallback (since ?? only catches null/undefined, not ""). It also removes duplicate validator functions in the flatpak, snap, and winget targets and tightens validation rules (e.g. Flatpak segments must start with a letter, snap names must not contain --).

  • agents.ts: Replaces result.stdout ?? result.stderr ?? '' with (result.stdout?.trim() || result.stderr?.trim()) ?? '' so that an empty-string stdout correctly falls through to stderr.
  • agents.test.ts: Adds a module-level vi.mock for spawnSync with beforeEach reset, and a new regression test confirming version parsing from stderr.
  • pkg-flatpak/snap/winget: Removes duplicate validator definitions, tightens regex/rule coverage, and adds validation to renderSnapcraftYaml and early in build/ship.

Confidence Score: 5/5

Safe to merge — the change is a narrow, well-tested fix to a stdout/stderr fallback expression, accompanied by validator cleanup that removes duplicate code without altering observable behaviour.

The core change is a single-line expression swap with clear semantics, backed by a dedicated regression test. Duplicate function removals in the target packages consolidate identical or overlapping logic with no missing call sites. All existing tests continue to pass and new ones specifically cover the fixed path.

No files require special attention.

Important Files Changed

Filename Overview
packages/cli/src/commands/agents.ts Core bug fix: replaces null-coalescing fallback with a truthy OR so empty-string stdout correctly falls through to stderr for version parsing.
packages/cli/src/commands/agents.test.ts Adds vi.mock for spawnSync with proper beforeEach reset/default and a new regression test for stderr version parsing; previous afterEach concern is resolved by the beforeEach mockReset.
packages/targets/pkg-flatpak/src/index.ts Removes duplicate validateAppId definition and tightens segment regex to require segments starting with a letter, matching Flatpak/D-Bus conventions.
packages/targets/pkg-snap/src/index.ts Removes duplicate validateSnapName, adds consecutive-hyphen check, and calls validation early in renderSnapcraftYaml (resulting in harmless double-validation in build, but correct defense-in-depth).
packages/targets/pkg-winget/src/index.ts Removes a duplicate validatePackageId definition; the authoritative implementation with stricter checks remains at the top of the file and is called from build and ship.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[probeAgent called] --> B[spawnSync binary --version]
    B --> C{result.error OR\nstatus !== 0?}
    C -- Yes --> D[Return installed: false]
    C -- No --> E{stdout.trim\ntruthful?}
    E -- Yes --> F[raw = stdout.trim]
    E -- No --> G{stderr.trim\ntruthful?}
    G -- Yes --> H[raw = stderr.trim]
    G -- No --> I[raw = '']
    F --> J[regex match semver]
    H --> J
    I --> J
    J --> K{match found?}
    K -- Yes --> L[version = captured group]
    K -- No --> M[version = first line of raw]
    L --> N[Return installed: true, version]
    M --> N
Loading

Reviews (3): Last reviewed commit: "Reset agents spawn mock between tests" | Re-trigger Greptile

@ayskobtw-lil
Copy link
Copy Markdown
Contributor Author

Update: the failing test workflow was the same repo-wide package-target duplicate validator issue already affecting other PRs, not the agents change. I folded in the package-validator cleanup so this PR can run the full suite independently.\n\nVerification after the update:\n- npx --yes pnpm@9.12.0 exec vitest run packages/cli/src/commands/agents.test.ts packages/targets/pkg-flatpak/src/index.test.ts packages/targets/pkg-winget/src/index.test.ts packages/targets/pkg-snap/src/index.test.ts -> 26 passed\n- npx --yes pnpm@9.12.0 --filter @profullstack/sh1pt --filter @profullstack/sh1pt-target-pkg-flatpak --filter @profullstack/sh1pt-target-pkg-winget --filter @profullstack/sh1pt-target-pkg-snap typecheck -> passed

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 1, 2026

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@ayskobtw-lil ayskobtw-lil force-pushed the codex/agents-version-stderr branch from 61545ab to 397b6a4 Compare June 1, 2026 15:40
@ayskobtw-lil
Copy link
Copy Markdown
Contributor Author

Follow-up update: I rebased this branch onto current master and addressed Greptile's test-isolation note by resetting the mocked spawnSync in beforeEach before applying the default mock return value.\n\nFresh verification after the rebase:\n- npx --yes pnpm@9.12.0 exec vitest run packages/cli/src/commands/agents.test.ts packages/targets/pkg-flatpak/src/index.test.ts packages/targets/pkg-winget/src/index.test.ts packages/targets/pkg-snap/src/index.test.ts -> 4 files / 26 tests passed\n- npx --yes pnpm@9.12.0 --filter @profullstack/sh1pt --filter @profullstack/sh1pt-target-pkg-flatpak --filter @profullstack/sh1pt-target-pkg-winget --filter @profullstack/sh1pt-target-pkg-snap typecheck -> passed

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

1 similar comment
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@ralyodio ralyodio merged commit 5bf7927 into profullstack:master Jun 2, 2026
5 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.

agents list drops versions printed to stderr

2 participants