fix(record): preserve env fallback prompt context#74
Merged
Conversation
Add dist CLI smoke coverage for public commands, Codex shell mutation classification tests, PR description/reviewer-context characterization tests, and Dashboard note merge robustness coverage. Release note: skip Agentnote-Session: 89e518cc-e1cc-46d7-bdd0-86525a708263
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds documentation updates and multiple tests: Codex shell-command classification, a built-CLI E2E smoke suite, post-commit/init/record env-fallback changes with tests, dashboard merge robustness, and PR-report edge-case tests. ChangesTest coverage expansion for CLI, dashboard, and PR reporting
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Document how regression tests should prefer unique scenarios, characterization coverage, isolated dist CLI smoke tests, and false-positive checks for fallback heuristics. Release note: skip Agentnote-Session: 89e518cc-e1cc-46d7-bdd0-86525a708263
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/commands/e2e-smoke.test.ts`:
- Around line 16-29: The test should fail fast when the built bundle is missing:
in the before() setup where cliPath is defined (variable cliPath) add an upfront
existence check (e.g., fs.existsSync or fs.accessSync) for the dist/cli.js file
and throw a clear error (or call this.skip()/assert.fail) if it does not exist
so the test suite exits with an explicit message instead of producing multiple
spawn errors later; update the before() block (where git(["init"]) is invoked)
to perform this check before creating the temp repo or spawning the CLI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cc5acefa-c681-4c5a-ad52-04a7e3e6e4d6
📒 Files selected for processing (6)
docs/engineering.mdpackages/cli/src/agents/codex.test.tspackages/cli/src/commands/e2e-smoke.test.tspackages/dashboard/workflow/sync-notes.test.mjspackages/pr-report/src/index.test.tspackages/pr-report/src/report.test.ts
wasabeef
changed the title
Why Env fallback was correctly recovering Codex/cmux commits, but direct file matching kept only the final implementation prompt. That made PR Report and Dashboard lose the investigation and decision prompts that explain why the change happened. User impact Codex env fallback notes now keep bounded preceding decision-context prompts for display while attribution and line counts still use only the direct file-matched transcript rows. Verification npm run build npm run typecheck npm run lint env -u CODEX_THREAD_ID node --import tsx/esm --test src/core/record.test.ts env -u CODEX_THREAD_ID node --import tsx/esm --test src/commands/init.test.ts env -u CODEX_THREAD_ID npm test Release note: Codex environment fallback now preserves useful prompt context without loosening AI attribution.
wasabeef
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/commands/e2e-smoke.test.ts`:
- Around line 120-121: The cleanup in the after() hook unconditionally calls
rmSync(testDir...) which throws when testDir was never created (e.g., before()
failed); update the after() hook to guard the removal by checking that testDir
is defined and exists before calling rmSync — reference the after() hook, the
before() assertion that sets testDir, and the testDir variable, and use an
existence check (e.g., existsSync or a truthy check) before invoking rmSync to
avoid a second exception during teardown.
- Around line 278-297: The helpers (git, gitOutput, gitIn, addNote) call
execFileSync("git", ...) directly which bypasses the package's canonical git
wrapper; replace those direct execFileSync usages with the exported git
execution helper from the package's git.ts (the canonical git execution
function), importing it at the top of the test file and calling it with the same
args, cwd and encoding/options so git(), gitOutput(), gitIn(), and addNote()
route all git operations through the shared wrapper instead of calling
execFileSync directly.
- Around line 255-260: The runCli function spawns CLI processes using the full
parent process.env which lets local AGENTNOTE_*, CODEX_*, CLAUDE_*, CURSOR_*,
GEMINI_* (and similar) variables leak into tests; change the spawnSync env
construction in runCli to build a minimal, hermetic environment instead of
spreading process.env — start from a clean object (or copy only a small
whitelist like PATH, NODE_ENV, HOME) set HOME and XDG_CONFIG_HOME as already
done, and explicitly omit or delete any variables with prefixes AGENTNOTE_,
CODEX_, CLAUDE_, CURSOR_, GEMINI_ (and any other agent-related prefixes) so the
spawned CLI cannot observe host agent/session state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8f46f198-8830-48e9-a55a-a64c74a3db07
📒 Files selected for processing (1)
packages/cli/src/commands/e2e-smoke.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
--fallback-envnotes instead of saving only the final file-matched implementation prompt.Release Note Plan
Release note: Codex environment fallback now preserves useful prompt context without loosening AI attribution.
Type of Change
Checklist
Notes for Reviewers
This PR started as regression-test hardening, then exposed two real recovery gaps:
--fallback-envfrom running even when the trailer path wrote no git note.The final shape intentionally separates display from attribution. Env fallback may keep bounded preceding transcript prompts so Agent Note preserves the “why,” but only direct file-matched transcript rows affect
files_touched, AI files, and line attribution.Local verification:
npm run buildinpackages/clinpm run typecheckinpackages/clinpm run lintinpackages/clienv -u CODEX_THREAD_ID node --import tsx/esm --test src/core/record.test.tsinpackages/clienv -u CODEX_THREAD_ID node --import tsx/esm --test src/commands/init.test.tsinpackages/clienv -u CODEX_THREAD_ID npm testinpackages/cli(458 tests)node packages/cli/dist/cli.js show HEADconfirms the latest commit records 13 prompts, including the cmux/fallback investigation context.Pre-existing local changes in
docs/knowledge/investigations.mdanddocs/blog/are not part of this PR.🧑💬🤖 Agent Note
Total AI Ratio: ████████ 100%
Model:
gpt-5.4296707ftest: strengthen regression coverage1b4aa3adocs: clarify regression test strategyeb73bfefix(hooks): retry env fallback after empty trailer notes5c67316fix(record): preserve env fallback prompt context7c33632test(cli): fail fast when dist smoke bundle is missing5c7f907test(cli): isolate dist smoke test environment💬 Prompts & Responses (20 shown / 22 total)
eb73bfefix(hooks): retry env fallback after empty trailer notes5c67316fix(record): preserve env fallback prompt context7c33632test(cli): fail fast when dist smoke bundle is missing5c7f907test(cli): isolate dist smoke test environmentSummary by CodeRabbit
Documentation
Tests
Improvements / Bug Fixes