Skip to content

Fill In the Release-Watch Timeout Placeholder - #944

Merged
ptr727 merged 1 commit into
developfrom
fix-release-watch-timeout
Aug 23, 2026
Merged

Fill In the Release-Watch Timeout Placeholder#944
ptr727 merged 1 commit into
developfrom
fix-release-watch-timeout

Conversation

@ptr727

@ptr727 ptr727 commented Aug 23, 2026

Copy link
Copy Markdown
Owner

Fixes the still-valid half of #939 (closed as superseded on its other half, checkout-safety, already fixed more thoroughly on develop via #937/#940).

Step 6's run-watch bound was a literal <seconds> template placeholder, never a runnable command as written. Filled in 2700 (45 minutes), matching scripts/pr_review.py's own default (verified: ap.add_argument("--timeout", type=int, default=2700, ...)).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Release monitoring now uses a finite 45-minute timeout by default.
    • Supports equivalent bounded waiting across different host environments.
    • Clearly distinguishes timed-out runs from completed results and generated versions.
    • Reports failed, timed-out, or unstarted runs without automatically retrying.
  • Documentation

    • Updated release monitoring guidance to reflect the new timeout and reporting behavior.

Step 6's run-watch bound was a literal '<seconds>' template
placeholder rather than a value, never a runnable command. Filled
in 2700 (45 minutes), matching scripts/pr_review.py's own default,
unless the maintainer states a different bound for a specific
release.

Originally caught by PR #939, from a separate session working the
same file concurrently. That PR's other fix (checkout-safety) is
superseded by the more thorough version already on develop from
#937/#940, so #939 was closed rather than merged, and this is the
one still-valid piece pulled out on its own.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 23, 2026 04:06
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ab756eec-ca44-4969-b39e-6fe09e15562d

📥 Commits

Reviewing files that changed from the base of the PR and between d00266a and 03f9b2c.

📒 Files selected for processing (4)
  • .agents/skills/merge-and-release/SKILL.md
  • .claude-plugin/fleet-skills/.source-digest
  • .claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md
  • .github/skills/merge-and-release/SKILL.md

Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

Release skill documentation now requires finite release-run waits, with a 2700-second default and platform-specific equivalents. It separates timeout status from run conclusions and produced versions, and reports unsuccessful or absent runs without retries.

Changes

Release-run monitoring

Layer / File(s) Summary
Bounded wait and outcome reporting
.agents/skills/merge-and-release/SKILL.md, .claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md, .github/skills/merge-and-release/SKILL.md, .claude-plugin/fleet-skills/.source-digest
The release skill copies define a 2700-second default timeout, allow maintainer-specified alternatives and platform-specific bounded waits, separate timeout status from conclusions and produced tags or versions, and prohibit retries for failed, timed-out, or absent runs. The source digest is updated.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 03f9b

This localized change replaces an unrunnable timeout placeholder with the intended 45-minute value; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: replacing the release-watch timeout placeholder with a concrete value.
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 files. (4 skipped: 4 unsupported.)
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-release-watch-timeout

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Replace release-watch timeout placeholder with 2700s default

📝 Documentation ⚙️ Configuration changes 🕐 Less than 5 minutes

Grey Divider

AI Description

• Replace the non-runnable timeout  placeholder with timeout 2700 guidance.
• Document the 45-minute bound and that maintainers may override it per release.
• Propagate the same wording across all mirrored merge-and-release skill docs and refresh the source
 digest.
Diagram

graph TD
  A["Maintainer / Agent"] --> B["Merge+Release SKILL.md"] --> C["CLI: timeout 2700"] --> D["GitHub Actions run"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Reference a single canonical timeout constant
  • ➕ Avoids drift if the default timeout changes elsewhere (e.g., in scripts/pr_review.py).
  • ➕ Keeps all docs in sync without manual copy updates.
  • ➖ Hard to express cleanly in a runbook that must be runnable as copy/paste shell.
  • ➖ May require extra explanation or tooling to resolve the value at runtime.

Recommendation: The PR’s approach (hardcoding 2700 with an explicit note that maintainers can override per release) is pragmatic for copy/paste runbook commands and fixes a real usability bug. If timeout defaults are expected to change over time, consider introducing a single canonical reference (doc include, generator, or shared constant) to reduce future drift across mirrored skill docs.

Files changed (4) +25 / -19

Documentation (3) +24 / -18
SKILL.mdMake 'gh run watch' timeout runnable (use 2700s) +8/-6

Make 'gh run watch' timeout runnable (use 2700s)

• Replaces the 'timeout <seconds>' placeholder with an explicit '2700' second bound and documents the 45-minute rationale and override guidance for specific releases.

.agents/skills/merge-and-release/SKILL.md

SKILL.mdMirror: set release-watch timeout to 2700s +8/-6

Mirror: set release-watch timeout to 2700s

• Applies the same documentation fix as the primary skill: replace the timeout placeholder with '2700' seconds and clarify override expectations.

.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md

SKILL.mdMirror: set release-watch timeout to 2700s +8/-6

Mirror: set release-watch timeout to 2700s

• Updates the GitHub-hosted copy of the skill documentation to use a concrete 45-minute timeout and matching explanatory text.

.github/skills/merge-and-release/SKILL.md

Other (1) +1 / -1
.source-digestRefresh fleet-skills source digest +1/-1

Refresh fleet-skills source digest

• Updates the digest value to reflect the modified skill content mirrored into the Claude plugin directory.

.claude-plugin/fleet-skills/.source-digest

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is a straightforward documentation/template fix consistently applied across source and generated outputs, with no remaining placeholder and an explicitly verified timeout default alignment.

Pull request overview

This PR replaces the timeout <seconds> placeholder in the merge-and-release Skill's release-watch step with a concrete bound (2700s / 45m), aligned with scripts/pr_review.py's --timeout default, and propagates the change through the generated skill distributions.

Changes:

  • Replace the non-runnable timeout <seconds> ... template with timeout 2700 ... and document the rationale (matches scripts/pr_review.py default).
  • Apply the same edit to the .agents/ source and both generated distributions (.github/skills/, .claude-plugin/fleet-skills/).
  • Update the fleet-skills source digest to reflect the new distribution content.
File summaries
File Description
.agents/skills/merge-and-release/SKILL.md Replaces the timeout placeholder with an explicit 2700s bound and explanatory text.
.github/skills/merge-and-release/SKILL.md Regenerated distribution copy reflecting the same timeout bound update.
.claude-plugin/fleet-skills/skills/merge-and-release/SKILL.md Regenerated plugin distribution copy reflecting the same timeout bound update.
.claude-plugin/fleet-skills/.source-digest Updates digest to match the regenerated plugin distribution content.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ptr727
ptr727 merged commit ec90781 into develop Aug 23, 2026
9 checks passed
@ptr727
ptr727 deleted the fix-release-watch-timeout branch August 23, 2026 04:10
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Capitalize bind word correctly 📘 Rule violation ⚙ Maintainability
Description
The PR title uses In in the middle of the title, but in is a designated lowercase bind word when
it is not first or last. Rename it to Fill in the Release-Watch Timeout Placeholder.
Code

.github/skills/merge-and-release/SKILL.md[R97-98]

+   2700 seconds (45 minutes, matching `scripts/pr_review.py`'s own default) unless the maintainer
+   states a different bound for this specific release: `timeout 2700 gh run watch <run-id> --repo
Evidence
The checklist requires short bind words including in to remain lowercase when they occur in the
middle of a PR title. The supplied PR title contains In as its second word; the changed
release-watch documentation confirms the PR's scope but cannot change the title metadata.

Rule 2826422: Enforce Title Case for Pull Request Titles with Lowercase Short Bind Words
.github/skills/merge-and-release/SKILL.md[97-98]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR title `Fill In the Release-Watch Timeout Placeholder` violates the title-case rule because `In` is a middle-position bind word that must be lowercase.

## Issue Context
The required title form is `Fill in the Release-Watch Timeout Placeholder`; the title otherwise remains imperative and within the character limit.

## Fix Focus Areas
- .github/skills/merge-and-release/SKILL.md[97-98]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 68 rules
✅ Skills: 5 invoked
  comment-and-doc-style
  dotnet-codestyle
  python-codestyle
  shell-codestyle
  workflow-ci-contract
Review mode: 🚀 Fast: This is a localized documentation/instruction change replicated across three copies, replacing a placeholder with a fixed timeout; it avoids runtime, security, and other high-risk behavior.

Grey Divider

Tip of the day
💡 Did you know, you can turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +97 to +98
2700 seconds (45 minutes, matching `scripts/pr_review.py`'s own default) unless the maintainer
states a different bound for this specific release: `timeout 2700 gh run watch <run-id> --repo

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Capitalize bind word correctly 📘 Rule violation ⚙ Maintainability

The PR title uses In in the middle of the title, but in is a designated lowercase bind word when
it is not first or last. Rename it to Fill in the Release-Watch Timeout Placeholder.
Agent Prompt
## Issue description
The PR title `Fill In the Release-Watch Timeout Placeholder` violates the title-case rule because `In` is a middle-position bind word that must be lowercase.

## Issue Context
The required title form is `Fill in the Release-Watch Timeout Placeholder`; the title otherwise remains imperative and within the character limit.

## Fix Focus Areas
- .github/skills/merge-and-release/SKILL.md[97-98]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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