Skip to content

fix(ci): remove stale hardcoded timeout-minutes claim from sidecar comment - #1510

Draft
seonghobae wants to merge 1 commit into
mainfrom
fix/sidecar-preflight-comment-stale-timeout-minutes
Draft

fix(ci): remove stale hardcoded timeout-minutes claim from sidecar comment#1510
seonghobae wants to merge 1 commit into
mainfrom
fix/sidecar-preflight-comment-stale-timeout-minutes

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Summary

scripts/ci/contextual_orchestrator_review_sidecar.sh's gateway-preflight comment (around line 431-432) claimed:

This required-workflow job already budgets 120 minutes (see timeout-minutes in strix.yml/noema-review.yml)

Both halves of that claim are now stale:

  • .github/workflows/strix.yml's job-level timeout-minutes is 200, not 120.
  • .github/workflows/noema-review.yml had no timeout-minutes at all until PR fix(noema): raise call_llm HTTP timeout from 120s to org policy #1509 gave it timeout-minutes: 300 (fixing a real bug where a slow LLM review, now allowed up to the org's documented ~2-hour-per-model policy, could outlive the GitHub App token used to submit its verdict).

Found while a sibling agent was fixing PR #1509 and cross-checked the timeout-budget claims in this file's comments against the actual workflow files.

Fix

Reworded the comment to reference each caller workflow's own job-level timeout-minutes without pinning a specific shared number, since the two workflows' budgets now differ (200 vs. 300) and can change independently without re-staling this comment again. No behavior change — this is a comment-only edit. The 120-second curl preflight timeout value itself (a distinct, deliberately-unchanged-by-ADR-0005 value) is untouched.

Developer experience

  • bash -n scripts/ci/contextual_orchestrator_review_sidecar.sh — syntax OK.
  • coverage run -m pytest tests -q — 2126 passed, 1 skipped, 21 subtests passed (unchanged from before this edit; no test pins this comment's exact text).
  • No *-hashes.txt file touched.

User experience

No production or CI behavior change. Purely prevents a future reader (human or agent) from trusting a stale, now-inaccurate timeout figure when reasoning about this script's preflight budget.

Test plan

  • bash -n scripts/ci/contextual_orchestrator_review_sidecar.sh
  • coverage run -m pytest tests -q — 2126 passed, 1 skipped, 21 subtests passed

Generated by Claude Code

…mment

The gateway-preflight comment in contextual_orchestrator_review_sidecar.sh
claimed "this required-workflow job already budgets 120 minutes (see
timeout-minutes in strix.yml/noema-review.yml)". Both halves of that claim
are now stale: strix.yml's job timeout-minutes is 200, not 120, and
noema-review.yml had no timeout-minutes at all until PR #1509 gave it
timeout-minutes: 300 (fixing a bug where a slow LLM review could outlive
the GitHub App token used to submit its verdict).

Reworded to reference each caller workflow's own job-level timeout-minutes
without pinning a shared number, since the two workflows' budgets now
differ and can change independently without re-staling this comment again.
Comment-only change; no behavior, test, or hash-pinned file touched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

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.

2 participants