fix: harden repo-local helper launcher resolution#90
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce4c11c1c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (result.error.code === 'ENOENT') return false; | ||
| throw result.error; | ||
| } | ||
| process.exit(result.status ?? 0); |
There was a problem hiding this comment.
Propagate child termination signals in helper launcher
spawnSync returns status: null and sets signal when the child is terminated, but process.exit(result.status ?? 0) converts that into exit code 0. As a result, node .agents/bin/gsdd.mjs ... can report success even when the underlying gsdd command was killed (for example by SIGTERM/OOM/timeout), which allows workflows to continue after failed file-op/phase-status mutations. Treat signaled exits as failures (or re-emit the signal) instead of coercing them to success.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the merged version. The generated workflow helper no longer proxies through a spawned child process; it dispatches the local helper commands directly from .planning/bin/gsdd.mjs, so the signal-to-success coercion path no longer exists.
| } | ||
|
|
||
| if (platforms.length > 0 || existsSync(join(ctx.cwd, '.agents', 'skills'))) { | ||
| if (platforms.length > 0 || existsSync(join(ctx.cwd, '.agents'))) { |
There was a problem hiding this comment.
Restrict portable-surface update trigger to GSDD markers
This condition now treats any existing .agents/ directory as evidence that GSDD portable surfaces are installed. In repos that use .agents/ for unrelated tooling, gsdd update will unexpectedly generate .agents/skills/gsdd-* and .agents/bin/gsdd.mjs even when gsdd init was never run, causing unintended repository mutations. The trigger should check for explicit GSDD-owned paths (for example .agents/bin/gsdd.mjs or gsdd-* skill folders) rather than the top-level .agents directory.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed on main in 1b89a6a. gsdd update no longer treats .agents/ or a generic .agents/skills/ directory as GSDD-owned; the portable-surface trigger now requires an existing .agents/skills/gsdd-*/SKILL.md path, while initialized projects still repair from .planning/. Added regression coverage for an unrelated .agents/skills/custom-agent/SKILL.md directory to ensure no GSDD skills or planning state are generated.
Summary
.agents/bin/gsdd.mjslauncher as part of the portable runtime surfacegsdd ...gsdd-cli@<version>gsdd updateRoot cause
Generated workflows assumed helper commands could call bare
gsdd ..., butgsdd initonly guaranteed generated files inside the repo, not an ambient PATH install. The first launcher draft still left two gaps: partial.agents/installs were not fully self-repaired bygsdd update, and ambient PATH resolution could drift to the wrong CLI version.Testing
node tests/gsdd.init.test.cjsnode tests/gsdd.manifest.test.cjsnode tests/phase.test.cjsnode tests/gsdd.scenarios.test.cjsnode tests/gsdd.health.test.cjsnode tests/gsdd.cross-runtime.test.cjsnode --test-name-pattern="Phase 18 deterministic CLI guards|G45 - Runtime Surface Freshness Contract|G10 - CLI Module Boundary" tests/gsdd.guards.test.cjsnode tests/gsdd.invariants.test.cjs