Skip to content

docs: add CONTRIBUTING notes for CI caching and subprocess test timeouts - #2322

Merged
jatmn merged 1 commit into
mainfrom
docs/contributing-caching-timeouts
Oct 22, 2026
Merged

jatmn merged 1 commit into
mainfrom
docs/contributing-caching-timeouts

Conversation

@jatmn

@jatmn jatmn commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Adds contributor guidance to CONTRIBUTING.md for two behaviors that landed without docs:
    • PR Checks dependency caching (ci: cache package downloads in PR checks #2313): cache keys come from bun.lock / web/bun.lock, installs stay --frozen-lockfile on cache hits, so lockfiles must stay in sync and the local preflight is still required regardless of cache status.
    • Timing-sensitive and subprocess tests (test: stabilize timing-sensitive tests #2316): await completion/readiness signals instead of racing delays, and give real-subprocess tests an explicit per-test timeout longer than the child deadline.
  • Why: these are rules contributors need to follow, and they were only discoverable from the PR diffs.

Impact

  • user-facing impact: none
  • developer/maintainer impact: clearer test-writing and dependency-change guidance; docs only.

Testing

  • I ran the required local preflight.
  • exact commands and results: git diff --check clean. Docs-only change, so no code checks are affected.
  • focused tests: n/a (docs only)
  • documented skipped checks, platform limitations, or verified pre-existing failures: full local preflight not run, since only CONTRIBUTING.md changes; CI covers the rest.

Notes

  • provider/model path tested: n/a
  • screenshots attached (if UI changed): n/a
  • follow-up work or known limitations: none

Summary by CodeRabbit

  • Documentation
    • Updated development guidance to recommend waiting for asynchronous readiness before making assertions, using bounded waits when readiness is intentionally suppressed, and setting timeouts for tests that launch subprocesses.
    • Expanded validation notes with details on pull request check caches and guidance to use frozen-lockfile installs and run local preflight checks regardless of cache status.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: Twigpine/openclaude/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Essentials
  • Run ID: cdc6ca85-2b74-48c3-bfd5-4a108408867a
📥 Commits

Reviewing files that changed from the base of the PR and between ae83f6e and 1fcee81.

📒 Files selected for processing: 1
  • CONTRIBUTING.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout: 4
  • GitHub Check: pr-intent-scan
  • GitHub Check: CodeQL (javascript-typescript)
  • GitHub Check: CodeQL (actions)
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions: 2
Review docs for accuracy against current code behavior.

⚙️ CodeRabbit configuration file

Files:

  • CONTRIBUTING.md
Apply the OpenClaude maintainer review rubric from AGENTS.md.

⚙️ CodeRabbit configuration file

Files:

  • CONTRIBUTING.md
🔇 Additional comments: 2
CONTRIBUTING.md: 2

163-164: LGTM!

312-312: 📐 Maintainability & Code Quality

The cache guidance matches .github/workflows/pr-checks.yml. No documentation update is needed.

📝 Walkthrough

CONTRIBUTING.md adds guidance for asynchronous test readiness and subprocess timeouts. It also documents PR Checks’ Bun download caches and reiterates validation steps that apply regardless of cache status.

Changes: Contributor workflow guidance

Layer / File(s) Summary
Asynchronous test guidance
CONTRIBUTING.md
The development workflow now covers awaiting readiness, bounded waits when readiness is suppressed, and subprocess test timeouts. The example sets an 8-second child deadline and a 10-second test timeout.
Validation and cache guidance
CONTRIBUTING.md
The validation notes describe OS-scoped Bun download caches, lockfile-based keys and restore prefixes, frozen-lockfile installs, and local preflight requirements.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Other

Suggested reviewers: gravirei

Merge Risk: ⚪ Minimal · up to 1fcee

The contributor guidance has no identified issue that needs resolution before merge.

Pre-merge checks: Passed 7
Check name Status Explanation
Title check Passed The title is concise, scoped, and accurately describes the documentation changes for CI caching and subprocess test timeouts.
Description check Passed The description follows the required template and clearly documents the changes, impact, testing status, and notes. It records that the required local preflight was not run because this is a documenta…
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
Risk Surface Disclosed Passed PASS — The PR changes only CONTRIBUTING.md. It adds guidance for test readiness, subprocess timeouts, and existing PR Checks dependency-cache behavior. It does not change auth, routing, permissions,…
No Hidden Policy Change Passed PASS — The PR changes only CONTRIBUTING.md and adds contributor guidance for async assertions, subprocess test timeouts, Bun cache behavior, frozen-lockfile installs, and local preflight requirement…
Full details: Description check

Explanation

The description follows the required template and clearly documents the changes, impact, testing status, and notes. It records that the required local preflight was not run because this is a documentation-only change.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

Full details: Risk Surface Disclosed

Explanation

PASS — The PR changes only CONTRIBUTING.md. It adds guidance for test readiness, subprocess timeouts, and existing PR Checks dependency-cache behavior. It does not change auth, routing, permissions, outbound network behavior, background execution, startup/configuration, plugins/MCP, CI permissions, or release scripts. No listed risk surface or blocker is introduced.

Full details: No Hidden Policy Change

Explanation

PASS — The PR changes only CONTRIBUTING.md and adds contributor guidance for async assertions, subprocess test timeouts, Bun cache behavior, frozen-lockfile installs, and local preflight requirements. The diff introduces no product behavior, trust-model, routing default, telemetry/network, or permission-policy decision. No maintainer alignment is required for this check.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@jatmn jatmn self-assigned this Oct 22, 2026

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

— jatmn's PR Review agent

Standalone documentation follow-up for merged #2313 and #2316. The three added lines in CONTRIBUTING.md match the workflow and test patterns I checked on this head. I do not see a code or docs defect in the review diff; clear the merge-readiness items below before landing.

Merge readiness

  • Rebase onto current main. Live main is 1c42a106 (fix(shell): honor an abort that lands before the listener is attached (#2321)). This head is one commit behind and mergeable_state is blocked while the PR remains a draft. Rebase (or merge main forward) so the branch is current before merge.
  • Mark the PR ready for review. GitHub still lists this as a draft; branch protection will keep the merge gate blocked until it is marked ready.
  • CI at head 1fcee819e0220b339da0bb4c4418cbda814dc95b: changes and pr-intent-scan succeeded. Markdown-only path filtering skipped smoke-and-tests, all eight unit-tests shards, dedicated-tests, typecheck, launcher jobs, and web (expected for a root .md-only diff per pr-checks.yml). CodeQL (actions) and Analyze (javascript-typescript) succeeded. Aggregate CodeQL is neutral (see automated review audit). At review time CodeQL (javascript-typescript) was still in_progress; wait for it to finish and confirm it does not fail before merge.
  • Preflight checkbox. The PR body leaves the local preflight box unchecked and notes git diff --check only. That is acceptable for this docs-only delta given CI ran pr-intent-scan, but align the Testing section with CONTRIBUTING.md Validation if you want the template to reflect the markdown-only CI skip policy explicitly.

Automated review audit

Source Thread / check ID Status on head Disposition
CodeRabbit Issue comment #6330084057 Summary comment only; review was in progress with no line findings Nothing to promote; no actionable request yet
CodeQL (GitHub Advanced Security) Check run 119576856726 neutral — missing javascript-typescript configuration on the PR ref Drift, not a PR defect: docs-only change; 0 open code-scanning alerts on refs/pull/2322/head; no code finding
CodeQL Actions / Analyze Runs on head success No alerts to address

Findings

🟢 Thanks for the contribution. I do not see any actionable issues from my review.

@jatmn
jatmn marked this pull request as ready for review October 22, 2026 01:06
@jatmn
jatmn merged commit 75c73ea into main Oct 22, 2026
14 checks passed
@jatmn
jatmn deleted the docs/contributing-caching-timeouts branch October 22, 2026 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant