Skip to content

docs(doctoring): verify loop-brief items 4, 15-18, 38, 39 already resolved - #1758

Merged
seonghobae merged 7 commits into
mainfrom
claude/workflow-orchestrator-improvements-b16824
Sep 3, 2026
Merged

docs(doctoring): verify loop-brief items 4, 15-18, 38, 39 already resolved#1758
seonghobae merged 7 commits into
mainfrom
claude/workflow-orchestrator-improvements-b16824

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Answers loop-brief items 15-18 (workflow consolidation / rate-limit /
installation-token-format check) with evidence rather than re-investigating
each from scratch. All four are already resolved on this branch's base
(4f95abc); this PR only records that.

Test plan

  • Docs-only change; no scripts/ci/ or workflow-YAML behavior touched.
  • Confirmed no test enumerates docs/doctoring/ as a whole (only
    specific named files), so adding this file cannot break a contract test.

🤖 Generated with Claude Code


Devin Review

org-queue-sweep can't be replaced by plain Actions syntax (no native
org-wide PR sweep primitive) and its rate-limit symptom traces to the
plan-level 60-job ceiling, not the job's own design; the governance-risk-
compliance caller cited in item 16 was already consolidated (#1673) and its
hardcode fixed (#1743); item 17's org-wide dedup audit already ran (#1731);
and no script or workflow in this repo assumes a fixed installation-token
length/prefix, so the incoming ghs_ format change (item 18) needs no change
here. Records the evidence so these aren't re-investigated from scratch.
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 50 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: e41e8839-c0cb-4e99-ba5e-052f962f118d

📥 Commits

Reviewing files that changed from the base of the PR and between 4f95abc and e68bb67.

📒 Files selected for processing (3)
  • .github/workflows/codeql-pr.yml
  • docs/doctoring/loop-brief-items-15-18-verification-20260903.md
  • tests/test_codeql_pr_workflow_contract.py

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.

devin-ai-integration[bot]

This comment was marked as resolved.

- Distinguish the plan-level 60-job Actions ceiling from the separate,
  already-documented NVIDIA NIM LLM-provider rate limit instead of
  collapsing both into one cause.
- Broaden the item-18 token-format search from scripts/ci/*.py and
  .github/workflows/*.yml to every .py/.sh under scripts/ci/ and .github/
  (including the one composite action) plus a whole-repo double-check.
- Drop backtick-wrapped `.github#N` references, which GitHub does not
  autolink inside code spans; use plain #N so they resolve on this repo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

The opaque 900-second Noema "Repair" wall-clock deadline (no telemetry on
why) was already root-caused and fixed same-day: the bound had no
owner-specified basis, and Noema was duplicating a repair/failover
responsibility contextual-orchestrator already owns. The fix moved to one
caller request with no fixed deadline plus per-attempt Actions telemetry,
enforced by a permanent contract test. Confirmed the test still passes on
this branch and that the item-39 example PR's post-fix retry is queued
behind the already-documented org job ceiling, not a repair-deadline
recurrence.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@seonghobae seonghobae changed the title docs(doctoring): verify loop-brief items 15-18 already resolved docs(doctoring): verify loop-brief items 4, 15-18, 39 already resolved Sep 3, 2026
The closing line said no workflow-file edit could address any of the three
rate-limit resources, but the paragraph right above it already describes
two workflow-level mitigations in place (strix.yml's per-repo serialization,
org-queue-sweep's own pagination/budget ceilings). Only the plan-level
60-job ceiling is actually outside a workflow file's reach; say that
precisely instead of a blanket claim the preceding text contradicts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

seonghobae and others added 2 commits September 3, 2026 10:40
…ences

Both mentions of the item-39 example PR omitted the ContextualWisdomLab/
owner prefix required for a cross-repository reference (this doc lives in
.github, contextual-orchestrator is a different repo) and one was wrapped
in backticks, which also blocks GitHub's autolinking. Use plain
ContextualWisdomLab/contextual-orchestrator#1008 in both places.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
New repositories already get automatic CodeQL coverage from their first
pull request via the org required-workflow ruleset's dynamic ~ALL target
(codeql-pr.yml), with no PR needing to be opened into the new repo at all.
The three repos on GitHub's native code-scanning default setup cover the
ruleset's own exclusions (noema needs it because it's excluded from the
ruleset; .github runs codeql-pr.yml directly; IRT-bibliography-set has
neither, consistent with being a non-code repo). This is a stronger answer
than the requested Noema/OpenCode-authored-PR mechanism, not a gap.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@seonghobae seonghobae changed the title docs(doctoring): verify loop-brief items 4, 15-18, 39 already resolved docs(doctoring): verify loop-brief items 4, 15-18, 38, 39 already resolved Sep 3, 2026

@devin-ai-integration devin-ai-integration Bot 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.

Devin Review found 1 new potential issue.

Devin Review

Comment on lines +58 to +60
".github", "IRT-bibliography-set"]}` — `~ALL` is a *dynamic* match, so a
brand-new repository is covered from its very first pull request with zero
manual or automated action, the moment that PR exists. `.github` runs

@devin-ai-integration devin-ai-integration Bot Sep 3, 2026

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.

🟡 New repositories can miss CodeQL

Repositories using another default-branch name or source language receive no applicable scan from codeql-pr.yml. The dynamic ruleset cannot provide universal coverage.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed and fixed both catches:

  1. The branch restriction was real and live-verified beyond the workflow text: j-planner's open PR 🛡️ Sentinel: Add organization-wide default security policy #2 (default branch gh-pages) received every other required check but zero CodeQL checks of any kind. Removed branches: [main, master, develop] from codeql-pr.yml's pull_request trigger, matching security-scan.yml's own "do not restrict the base ref" precedent, and updated the one contract-test assertion that pinned the old line.
  2. The language-matrix gap (missing C/C++, C#, Go, Ruby, Swift) is correct too, but it's a larger change (new per-language detection heuristics + build-mode handling + test fixtures) than fits this PR -- filed as codeql-pr.yml language-detection matrix misses C/C++, C#, Go, Ruby, Swift #1762 rather than rushing it in.

🤖 Addressed by Claude Code

…ranches

codeql-pr.yml's own `branches: [main, master, develop]` filter sat
underneath the org required-workflow ruleset's dynamic ~DEFAULT_BRANCH
target, silently producing zero CodeQL checks for any repository whose
default branch isn't literally one of those three names. Confirmed live:
j-planner (default branch gh-pages) received every other required check on
its open PR #2 but no CodeQL check at all; argos/OmniRoute/graphify (forks
defaulting to developmental/release-v3.8.50/v8) were equally exposed.

Removed the restriction to match security-scan.yml's own established
"do not restrict the base ref" precedent -- the ruleset's ref_name condition
is already the authoritative gate, so the hardcoded list was redundant risk,
not a second intended layer. Updated the one contract-test assertion that
pinned the old line; actionlint and the rest of the workflow's/repo's tests
are unaffected.

Filed the language-detection matrix gap Devin also caught (misses C/C++,
C#, Go, Ruby, Swift) as .github#1762 -- larger, separately-scoped change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@seonghobae
seonghobae merged commit bf5970d into main Sep 3, 2026
7 of 26 checks passed
@seonghobae
seonghobae deleted the claude/workflow-orchestrator-improvements-b16824 branch September 3, 2026 02:35
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