Repository navigation
fix(repro-yaml): export env vars before bun -e (bash positional bug) - #1204
Conversation
Two occurrences of `bun -e "..." KEY="$VAL"` in
`scripts/tests/repro-yaml-config.sh` had the env-var in a TRAILING
position, which bash treats as a positional argument to bun — not as
an environment assignment — so `process.env.KEY` was always
undefined inside the script.
First symptom: `TypeError: undefined is not an object (evaluating
'xs.map')` from postgres.js when it tried to serialize `undefined`
as an identifier in `CREATE SCHEMA ${sql(process.env.TEST_SCHEMA)}`.
Second symptom (once past TEST_SCHEMA): `ENOENT: no such file or
directory, lstat 'WORK=/tmp/repro-yaml-XXXX'` because `WORK="$WORK"`
was picked up as a positional path arg by `builtin-agents.ts`'s
package-root resolver.
Fix: export both `TEST_SCHEMA` and `WORK` before the first bun
invocation. Drop the trailing `KEY=VAL` positionals from the two
downstream `bun -e` calls.
Verified: `bash scripts/tests/repro-yaml-config.sh` → "ALL INVARIANTS
HOLD", migration + yaml-edit-to-DB propagation both land.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request refactors the repro-yaml-config.sh script to ensure environment variables like TEST_SCHEMA and WORK are correctly inherited by bun -e child processes. By exporting these variables globally rather than passing them as trailing arguments, the script avoids bash interpreting them as positional arguments. A review comment suggests moving the TEST_SCHEMA export even earlier in the script to ensure that the cleanup trap can successfully drop the schema if the script fails prematurely.
| # `KEY=VALUE` after `bun -e "..."` does NOT get passed through to the | ||
| # child process's env (bash treats it as a positional arg). Exporting | ||
| # up front is the only form that actually makes it into the script. | ||
| export TEST_SCHEMA="$SCHEMA" |
There was a problem hiding this comment.
The export of TEST_SCHEMA should ideally be moved to line 30 (immediately after SCHEMA is defined). This ensures that the cleanup trap (set at line 53) can correctly access process.env.TEST_SCHEMA and drop the schema even if the script fails or is interrupted before reaching this line. Currently, an early exit would cause the bun call in the cleanup function to fail because the environment variable is not yet set.
Follow-up to PR #1201 (G7 of wish `dir-sync-frontmatter-refresh`). One-file, 13-line fix.
Symptom
Running `bash scripts/tests/repro-yaml-config.sh` against v4.260418.19 crashed twice:
Cause
Two `bun -e "..."` invocations had `KEY="$VAL"` trailing the command. Bash only treats `KEY=VAL` as an env assignment when it prefixes a command — trailing is a positional argument. So `process.env.KEY` was always undefined inside the bun script, and the literal `KEY=VAL` string got picked up as the first positional arg to whichever binary the script reached.
Fix
Export `TEST_SCHEMA` and `WORK` before the first `bun -e` call. Drop the trailing `KEY=VAL` from the two downstream invocations.
Verification
```
$ bash scripts/tests/repro-yaml-config.sh
...
✓ first sync migrated alice, no 'Unchanged' literal
✓ agent.yaml present, .bak present, AGENTS.md is body-only
✓ edited agent.yaml: model sonnet → opus
✓ second sync propagated yaml edit to DB (model=opus)
==> ALL INVARIANTS HOLD
```
Test plan
🤖 Generated with Claude Code