Skip to content

feat: implement issue #2316 — [ADR-0007] Sync the markets pilot package to markets#523 (ci-failure-analyst → v1-stable, self-exclusion guard fix) - #2317

Merged
don-petry merged 4 commits into
mainfrom
dev-lead/issue-2316-20261012-0144
Oct 12, 2026
Merged

don-petry merged 4 commits into
mainfrom
dev-lead/issue-2316-20261012-0144

Conversation

@don-petry

@don-petry don-petry commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

[ADR-0007] Sync the markets pilot package to markets#523 (ci-failure-analyst → v1-stable, self-exclusion guard fix)

From the issue: combsmith/markets#523 merged on 2026-10-12 as 9310b6e60001b9934f649c4ebe0f4abcdf3c3097. It changed one job in markets' collapsed .github/workflows/agent-ingress.yml (new blob a0c97e13afcbb8f8cd91f7b264327295e4db9b08), the ci-failure-analyst job:

Risk

Low — 5 file(s) changed; no CI- or runtime-critical paths touched.

Test plan

Tests added/updated: tests/agent_ingress_markets_package.bats,tests/fixtures/agent-ingress/markets-agent-ingress.yml tests/fixtures/agent-ingress/markets-pinned-reusables/ci-failure-analyst-reusable.yml,tests/fleet_stub_registry.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

This PR's CI checks (Lint + bats) are the success/regression signal.

Closes #2316

View guided diff Turn on auto-fix

…ge to markets#523 (ci-failure-analyst → v1-stable, self-exclusion guard fix)
@don-petry
don-petry requested a review from a team as a code owner October 12, 2026 01:57
@don-petry don-petry added the auto-rebase:ready Opts a non-draft PR into auto-rebase without an approval (auto-rebase ready_label) label Oct 12, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-12T02:30:37.999341Z 9ed01d8 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

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

Next included review available in 6 minutes.

Check out review usage here.

View limit details

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 82e18466-9a60-4878-bfc0-00560293aeda

📥 Commits

Reviewing files that changed from the base of the PR and between 9de8950 and 9ed01d8.


📒 Files selected for processing (5)
  • docs/initiatives/agent-ingress-collapse-markets.md
  • tests/agent_ingress_markets_package.bats
  • tests/fixtures/agent-ingress/markets-agent-ingress.yml
  • tests/fixtures/agent-ingress/markets-pinned-reusables/ci-failure-analyst-reusable.yml
  • tests/fleet_stub_registry.bats

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the ci-failure-analyst workflow configuration and documentation to resolve a defective pin by repointing it to a stable channel (@ci-failure-analyst/v1-stable) and correcting its self-exclusion guard prefix to 'ci-failure-analyst'. It also updates the pr-review pin to @pr-review/v1-stable and adds a new test to verify these changes. The feedback suggests enhancing the test assertions by explicitly verifying that the output of grep -c is 0 to ensure a more robust check.

Comment thread tests/agent_ingress_markets_package.bats
@codeant-ai

codeant-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

CodeAnt PR Risk: Low Risk

  • The PR appears safe to merge; it updates the documented ingress and test fixtures to reflect the reported markets#523 changes.
  • The regression test checks that the analyst's check-run name matches its self-exclusion prefix and that no defective-pin marker remains.

Assessed commit: 3edfcf2e485c

@don-petry

Copy link
Copy Markdown
Collaborator Author

QA Lead — test-risk advisory

Risk tier: LOW. The PR only touches docs, tests and fixtures, and only re-syncs an offline snapshot to markets#523, which has already merged. No runtime path in this repo changes, and reverting the PR undoes it completely.

What I'd shore up (highest leverage first):

  • Add a negative assertion for the guard. The new test checks that the ci-failure-analyst prefix matches ci-failure-analyst / analyze. It never checks that the old 'CI Failure Analyst' prefix is gone. That old prefix is the actual ci-failure-analyst caller: the self-exclusion guard never matches a real check name, and no-PR failures still start the analyst #2175 regression, so pin its absence directly, in both the §3 extraction and the fixture.
  • Assert the reusable has exactly one job. .jobs | keys | .[0] silently uses the first job if a second one is ever added to the snapshot. Check length == 1 before reading the job id.
  • Check that doc §3 and the fixture agree. fleet_stub_registry.bats reads the fixture, while the package suite reads the extracted §3. I found no test that keeps the two in step, so the doc can drift from the verbatim markets blob (a0c97e13) without any test failing. A YAML-equivalence check, or a git hash-object match against the recorded blob, would close that.
  • Be aware the snapshot can go stale. The analyze job id now sits behind a moving channel ref (ci-failure-analyst/v1-stable), and the suite is offline by design. If upstream renames the job, the self-exclusion guard breaks silently. That is acceptable for now, but whoever refreshes the snapshot should know it is a manual step.

Escalate? No.

I couldn't run the suites myself because bats isn't installed in my environment, so CI is the only verification.

Advisory only — I comment, I do not change code. Ground: BMAD Test Architecture (bmad-tea v1.19.0). Opt out on this item with the qa-lead:hands-off label.

@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-12T03:02:30Z.

@don-petry don-petry added the dev-lead:hands-off Exclude from dev-lead agent automation label Oct 12, 2026 — with Claude
…ecks

Addresses the review nit on #2317: status 1 alone could also come from a
grep error; the printed count of 0 confirms there was no match.

Refs #2316

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Muchy4bnEhyVEnd89TUtjj
@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-12T03:19:21Z.

…ard's dead prefix out

From the QA advisory on #2317:
- assert the extracted §3 is byte-identical to the markets fixture, so the doc
  and the fixture fleet_stub_registry.bats reads cannot drift apart;
- assert the old 'CI Failure Analyst' prefix (#2175) is gone;
- assert the snapshot reusable has exactly one job before reading its id.

Refs #2316

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Muchy4bnEhyVEnd89TUtjj
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (applied)

Changes committed and pushed. Requested items addressed:

  • tests/agent_ingress_markets_package.bats:164 — applied

@don-petry
don-petry enabled auto-merge October 12, 2026 02:28
@sonarqubecloud

Copy link
Copy Markdown

@don-petry

Copy link
Copy Markdown
Collaborator Author

dev-lead is withholding action on this item.

It is labeled dev-lead:hands-off (deliberately kept off-limits to dev-lead (e.g. changes to the dev-lead automation itself)), so dev-lead will not pick it up while that label is present. This notice is posted once so the withhold is visible rather than looking like a stalled run.

To re-enable automated pickup: remove the dev-lead:hands-off label.

@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-12T03:29:51Z.

@don-petry
don-petry merged commit 7437f7b into main Oct 12, 2026
51 of 54 checks passed
@don-petry
don-petry deleted the dev-lead/issue-2316-20261012-0144 branch October 12, 2026 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-rebase:ready Opts a non-draft PR into auto-rebase without an approval (auto-rebase ready_label) dev-lead:hands-off Exclude from dev-lead agent automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ADR-0007] Sync the markets pilot package to markets#523 (ci-failure-analyst → v1-stable, self-exclusion guard fix)

3 participants