Skip to content

fix(scripts): add short help to stale CI cleanup#3222

Merged
senamakel merged 1 commit into
tinyhumansai:mainfrom
alexzhu0:codex/OH-2611-cancel-stale-ci-short-help
Jun 2, 2026
Merged

fix(scripts): add short help to stale CI cleanup#3222
senamakel merged 1 commit into
tinyhumansai:mainfrom
alexzhu0:codex/OH-2611-cancel-stale-ci-short-help

Conversation

@alexzhu0
Copy link
Copy Markdown
Contributor

@alexzhu0 alexzhu0 commented Jun 2, 2026

Summary

  • Adds -h as a short help alias for scripts/cancel-stale-pr-ci.mjs.
  • Updates the usage text to document both -h and --help.
  • Adds a focused Node test proving help exits before invoking gh.

Problem

  • Adjacent contributor helper CLIs support both --help and -h, but cancel-stale-pr-ci.mjs only accepted --help.
  • The script depends on GitHub CLI for normal operation, so help paths must remain side-effect-free and usable without GitHub access.

Solution

  • Treat -h the same as --help before parsing action flags or calling GitHub CLI.
  • Test both aliases with a temporary failing gh stub in PATH so any accidental GitHub CLI invocation fails the test.

Submission Checklist

If a section does not apply to this change, mark the item as N/A with a one-line reason. Do not delete items.

  • Tests added or updated (happy path + failure guard: help exits 0 and fails if gh is invoked).
  • N/A: Diff coverage gate was not run because this is a root helper script change and local Rust tooling is unavailable.
  • N/A: Coverage matrix not affected; this does not add, remove, or rename a product feature.
  • N/A: No affected feature IDs; contributor maintenance script only.
  • No new external network dependencies introduced.
  • N/A: Manual smoke checklist not affected; this does not touch release-cut surfaces.
  • N/A: Related to [Automated] Weekly code-review report — 2026-05-25 #2611 but does not close it by itself.

Impact

  • Runtime/platform impact: none.
  • Developer impact: node scripts/cancel-stale-pr-ci.mjs -h now prints usage and exits without requiring gh.

Related


AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: codex/OH-2611-cancel-stale-ci-short-help
  • Commit SHA: 16b6ddb

Validation Run

  • pnpm --filter openhuman-app format:check (blocked after Prettier passed; see Validation Blocked)
  • pnpm typecheck: N/A, root Node helper-only change
  • Focused tests: node --test scripts/__tests__/cancel-stale-pr-ci-help.test.mjs
  • Rust fmt/check (if changed): N/A, no Rust files changed
  • Tauri fmt/check (if changed): N/A, no Tauri files changed
  • node scripts/codex-pr-preflight.mjs --lightweight
  • git diff --check --cached

Validation Blocked

  • command: pnpm --filter openhuman-app format:check
  • error: sh: cargo: command not found
  • impact: Prettier passed, but the Rust format phase cannot run in this local environment until Cargo is installed.

Behavior Changes

  • Intended behavior change: -h now prints usage and exits 0 for cancel-stale-pr-ci.mjs.
  • User-visible effect: contributor-facing maintenance CLI help is easier to discover.

Parity Contract

  • Legacy behavior preserved: --help, normal dry-run mode, --execute, and invalid argument behavior are unchanged.
  • Guard/fallback/dispatch parity checks: focused test verifies both help aliases exit before any gh invocation.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): none found in the current open PR list during preflight.
  • Canonical PR: this PR
  • Resolution (closed/superseded/updated): N/A

Summary by CodeRabbit

  • New Features

    • The script now supports -h as a shorthand alternative to --help for displaying usage and help information.
  • Tests

    • Added comprehensive test coverage to verify that both -h and --help flags work correctly and that the script displays proper help information without unnecessary operations.

Constraint: contributor maintenance scripts should expose help without requiring GitHub CLI calls.\nRejected: documenting only --help | the repo already supports short help aliases on adjacent helper scripts.\nConfidence: high\nScope-risk: narrow\nDirective: keep maintenance-script help paths before network or gh-dependent work.\nTested: node --test scripts/__tests__/cancel-stale-pr-ci-help.test.mjs; node scripts/codex-pr-preflight.mjs --lightweight; git diff --check --cached; pnpm --filter openhuman-app format:check reached cargo blocker after Prettier passed.\nNot-tested: full Rust format because cargo is not installed locally.
@alexzhu0 alexzhu0 requested a review from a team June 2, 2026 15:02
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 2, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This pull request adds a short -h flag as an alias to the existing --help option in the cancel-stale-pr-ci.mjs script, updating both the argument parser and usage text. A new test file verifies that both flags exit cleanly with appropriate help output without invoking external tools.

Changes

Help flag enhancement

Layer / File(s) Summary
Short help flag support
scripts/cancel-stale-pr-ci.mjs
Script help/usage message updated to display -h, --help and argument parsing logic expanded to treat -h equivalent to --help, triggering immediate exit with usage text.
Help flag test coverage
scripts/__tests__/cancel-stale-pr-ci-help.test.mjs
Test file verifies both -h and --help flags cause successful exit with expected usage content on stdout and no stderr output, confirming help handling bypasses external gh command invocation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested labels

working

Poem

A rabbit hops with flags in hand,
-h now joins the help command,
With tests to prove the exit's clean,
The help flag's the best ever seen! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a short -h help flag to the stale CI cleanup script. It is concise, specific, and clearly conveys the primary modification.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added the working A PR that is being worked on by the team. label Jun 2, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
scripts/__tests__/cancel-stale-pr-ci-help.test.mjs (1)

13-18: ⚡ Quick win

Consider cleaning up the temporary directory.

The mkdtempSync creates a directory that is never removed, leaving test artifacts in the system temp directory. Consider adding cleanup logic.

♻️ Proposed fix to add cleanup
 function runHelp(flag) {
   const binDir = fs.mkdtempSync(join(os.tmpdir(), 'openhuman-gh-stub-'));
-  fs.writeFileSync(
-    join(binDir, 'gh'),
-    '#!/usr/bin/env sh\necho "gh should not run for help" >&2\nexit 99\n',
-    { mode: 0o755 },
-  );
+  try {
+    fs.writeFileSync(
+      join(binDir, 'gh'),
+      '#!/usr/bin/env sh\necho "gh should not run for help" >&2\nexit 99\n',
+      { mode: 0o755 },
+    );
 
-  return spawnSync(process.execPath, [SCRIPT, flag], {
-    encoding: 'utf8',
-    env: {
-      ...process.env,
-      PATH: `${binDir}${process.platform === 'win32' ? ';' : ':'}${process.env.PATH ?? ''}`,
-    },
-  });
+    return spawnSync(process.execPath, [SCRIPT, flag], {
+      encoding: 'utf8',
+      env: {
+        ...process.env,
+        PATH: `${binDir}${process.platform === 'win32' ? ';' : ':'}${process.env.PATH ?? ''}`,
+      },
+    });
+  } finally {
+    fs.rmSync(binDir, { recursive: true, force: true });
+  }
 }
🤖 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/__tests__/cancel-stale-pr-ci-help.test.mjs` around lines 13 - 18, The
test creates a temporary directory with fs.mkdtempSync assigned to binDir and
writes a stub 'gh' there but never removes it; add cleanup to remove the temp
directory (and contents) after the test runs — e.g., in an afterEach/afterAll or
finally block, call fs.rmSync(binDir, { recursive: true, force: true }) or
equivalent to delete binDir and its 'gh' file so the temp artifact is not left
behind (update the test in cancel-stale-pr-ci-help.test.mjs where binDir is
created).
🤖 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.

Nitpick comments:
In `@scripts/__tests__/cancel-stale-pr-ci-help.test.mjs`:
- Around line 13-18: The test creates a temporary directory with fs.mkdtempSync
assigned to binDir and writes a stub 'gh' there but never removes it; add
cleanup to remove the temp directory (and contents) after the test runs — e.g.,
in an afterEach/afterAll or finally block, call fs.rmSync(binDir, { recursive:
true, force: true }) or equivalent to delete binDir and its 'gh' file so the
temp artifact is not left behind (update the test in
cancel-stale-pr-ci-help.test.mjs where binDir is created).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6ea7206e-3828-431f-95f1-6e5d24b36ba6

📥 Commits

Reviewing files that changed from the base of the PR and between 873a74e and 16b6ddb.

📒 Files selected for processing (2)
  • scripts/__tests__/cancel-stale-pr-ci-help.test.mjs
  • scripts/cancel-stale-pr-ci.mjs

@senamakel senamakel merged commit 6ba875a into tinyhumansai:main Jun 2, 2026
23 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants