fix(ops): make recording exporters honor --help without writing artifacts - #40
fix(ops): make recording exporters honor --help without writing artifacts#40nish3451 wants to merge 3 commits into
Conversation
…acts The recording queue, cockpit, teleprompter, and rehearsal-check exporters ignored --help/-h and generated their artifacts anyway. Each now handles --help/-h before doing any work: it prints usage and exits 0 without writing or overwriting any recording artifact (and without spawning draft-prospect-message in the cockpit exporter). Extend test-active-operator-surfaces.mjs with a --help surface for all four recording exporters (exit 0 + usage + no artifact overwrite).
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
📝 WalkthroughWalkthroughThe recording exporters now support ChangesRecording exporter help
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Note: the |
![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 `@scripts/test-active-operator-surfaces.mjs`:
- Around line 104-108: Expand the help checks in the recordingHelpSurface loop
to invoke each exporter with both --help and -h, asserting successful exit and
usage output for each alias. Add assertions for the exporter-specific options,
including --html for rehearsal, so the integration test validates the complete
advertised help surface rather than only Usage:.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0971d036-98e7-466b-8a9f-1b82ddcddd34
📒 Files selected for processing (5)
scripts/export-recording-cockpit.mjsscripts/export-recording-queue.mjsscripts/export-recording-rehearsal-check.mjsscripts/export-recording-teleprompter.mjsscripts/test-active-operator-surfaces.mjs
| for (const [name, artifactPaths] of recordingHelpSurface) { | ||
| for (const path of artifactPaths) writeFileSync(join(T, path), "help sentinel\n") | ||
| const helped = run([`scripts/${name}`, "--help"]) | ||
| eq(helped.status, 0, `${name} --help must exit 0: ${helped.stderr || helped.stdout}`) | ||
| mat(helped.stdout, /Usage:/, `${name} --help must print usage`) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Exercise both help aliases in this integration check.
The test invokes only --help, so the -h branch can regress without failing this check. Run each exporter with both aliases. Also assert the exporter-specific options, including --html for rehearsal, instead of checking only the Usage: prefix.
Proposed test matrix
for (const [name, artifactPaths] of recordingHelpSurface) {
- for (const path of artifactPaths) writeFileSync(join(T, path), "help sentinel\n")
- const helped = run([`scripts/${name}`, "--help"])
- eq(helped.status, 0, `${name} --help must exit 0: ${helped.stderr || helped.stdout}`)
- mat(helped.stdout, /Usage:/, `${name} --help must print usage`)
- for (const path of artifactPaths) {
+ for (const helpArg of ["--help", "-h"]) {
+ for (const path of artifactPaths) writeFileSync(join(T, path), "help sentinel\n")
+ const helped = run([`scripts/${name}`, helpArg])
+ eq(helped.status, 0, `${name} ${helpArg} must exit 0: ${helped.stderr || helped.stdout}`)
+ mat(helped.stdout, /Usage:/, `${name} ${helpArg} must print usage`)
+ for (const path of artifactPaths) {
deq(readFileSync(join(T, path), "utf8"), "help sentinel\n", `${name} --help must not overwrite ${path}`)
unlinkSync(join(T, path))
+ }
}
}🤖 Prompt for 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.
In `@scripts/test-active-operator-surfaces.mjs` around lines 104 - 108, Expand the
help checks in the recordingHelpSurface loop to invoke each exporter with both
--help and -h, asserting successful exit and usage output for each alias. Add
assertions for the exporter-specific options, including --html for rehearsal, so
the integration test validates the complete advertised help surface rather than
only Usage:.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
The fleet keeps dispatching the same finding to multiple lanes, producing duplicate same-fix PR pairs: #36/#44 and #55/#56 are byte-identical or near-identical patches of the operator export --help fix, #39/#49 the offername article fix, #40/#52 the recording exporter --help fix, plus #38/#48/#51, #30/#61/#72, #45/#64, #42/#60, #43/#54 and #46/#74. Add a PR Duplicate Guard: - scripts/check-pr-duplicates.mjs compares a PR's diff against every other open PR (shared changed-file coverage >= 0.8 and patch similarity >= 0.5). Calibrated on all 79 open PRs on 2026-08-11: every pair above the thresholds was a genuine duplicate-cluster member, zero false positives. - .github/workflows/pr-duplicate-guard.yml runs it on every PR event and posts one marker comment naming the duplicate(s) and the canonical PR; the check fails loudly when a duplicate is found. Informational, not required. - scripts/test-pr-duplicates.mjs covers parsing, similarity, detection, and comment upsert with an injected API; wired into npm ci and npm test.
|
Correction to my closing comment above: the keeper for this cluster is #160, not #56. I had wrongly concluded that This PR stays closed either way — its content is superseded by what is already on |
…main by PR #80 (duplicate guard) (#234) The duplicate-PR guard (PR #80, merged 2026-08-19) is now live on main, preventing the same-fix duplication pattern at the CI level. The named duplicate pairs (#36/#44, #39/#49, #40/#52) are functionally superseded: their underlying fixes are on main via PRs #135, #145, and #178. The prior 2026-08-15 lane run had concluded the guard existed but was stuck unmerged; on 2026-08-19 it landed and the item is resolved at the root-cause level. Verification-only run (no product or test files touched): - PR #80 source commit 2091c7a and merge commit 0a9909b are ancestors of origin/main - scripts/test-pr-duplicates.mjs → 'test-pr-duplicates: ok' (exit 0) - Superseding PRs #135 (d4f3ef4), #145 (fc44b42), #178 (77f6922) all on main - Guard workflow runs on pull_request events; not a required status, so existing work never blocks Co-authored-by: minimax-vps <minimax-vps@nish3451.dev>
1 participant
What
The four recording exporters (
prospect:queue,prospect:cockpit,prospect:teleprompter,prospect:rehearsal) ignored--help/-hand generated their artifacts anyway. Each now handles--help/-hbefore doing any work: it prints usage and exits 0 without writing or overwriting any recording artifact (and without spawningdraft-prospect-messagefrom the cockpit exporter).Files changed
scripts/export-recording-queue.mjsscripts/export-recording-cockpit.mjsscripts/export-recording-teleprompter.mjsscripts/export-recording-rehearsal-check.mjsscripts/test-active-operator-surfaces.mjs— adds a--helpsurface for all four exporters: exit 0, usage printed, and the default artifacts are left untouched (sentinel no-overwrite check).Validation
node scripts/<exporter> --helpand-hexit 0, printUsage:, and create noprospects/artifacts (verified live in a checkout with noprospects/dir).node scripts/test-active-operator-surfaces.mjspasses (covers both the new--helpsurface and the normal regeneration path throughprepare-recording-batch).npm testpasses (exit 0, no failures).Summary by CodeRabbit
New Features
--helpand-hoptions to recording export commands.Tests