Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/add-to-project.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ jobs:
add-to-project:
permissions:
contents: read
uses: petry-projects/.github/.github/workflows/add-to-project-reusable.yml@add-to-project/v1-stable # NOSONAR(githubactions:S7637) first-party channel ref
uses: petry-projects/.github/.github/workflows/add-to-project-reusable.yml@add-to-project/v1-ring1 # NOSONAR(githubactions:S7637) first-party channel ref
with:
project_id: PVT_kwDOD2inqs4BZq3-
project_url: https://github.com/orgs/petry-projects/projects/1
agent_ref: add-to-project/v1-stable
agent_ref: add-to-project/v1-ring1
# Pass only the secrets the reusable declares (least privilege; avoids
# `secrets: inherit` handing the reusable every org secret).
secrets:
Expand Down
63 changes: 62 additions & 1 deletion .github/workflows/initiative-driver.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@
# sub-issues to dev-lead — so the central driver does not have to watch every
# enrolled repo's events.
#
# Rate-limit gate (#640): before dispatching, this stub asks the source-side
# agent-rate-limit gate (petry-projects/.github/scripts/agent-rate-limit-gate.sh)
# whether `initiative-driver` may dispatch right now. initiative-driver is the
# canary that ENFORCES: the gate derives concurrency / cooldown / daily-budget /
# consecutive-failure counters from this stub's own run history (no state
# backend), reads every threshold from standards/agent-rate-limits.json, and
# serializes a close-event + schedule burst AHEAD of the cancel-in-progress
# concurrency group so two dispatches cannot race into cancellation (#443/#402).
# A `defer` simply skips the dispatch step — never a cancel, never a job failure.
#
# To adopt:
# 1. Copy this file verbatim to .github/workflows/initiative-driver.yml in your repo.
# 2. Ensure the `initiative:auto` label exists on the repo.
Expand Down Expand Up @@ -76,11 +86,62 @@ jobs:
GH_TOKEN: ${{ secrets.GH_PAT_DON_PETRY || secrets.GH_PAT_WORKFLOWS }}
run: |
if [ -z "${GH_TOKEN}" ]; then
echo "::error::GH_PAT_WORKFLOWS is required — a workflow_dispatch fired with GITHUB_TOKEN never starts a run."
echo "::error::GH_PAT_DON_PETRY (or legacy GH_PAT_WORKFLOWS) is required — a workflow_dispatch fired with GITHUB_TOKEN never starts a run."
exit 1
fi

- name: Checkout agent-rate-limit gate tooling
# Phase-4 wiring (#640): the gate library + orchestrator live in
# petry-projects/.github; an enrolled caller stub does not carry them, so
# fetch them read-only into a scratch path. Uses the same PAT as the
# dispatch — the workflow's GITHUB_TOKEN cannot read another repo — so the
# stub's `permissions:` block is unchanged.
# continue-on-error: a tooling-checkout outage must not stop the fleet —
# the gate step then finds no script, emits no decision, and the dispatch
# step's `!= 'defer'` guard falls through to dispatching (fail-open).
continue-on-error: true
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
repository: petry-projects/.github
ref: v1
path: .arl-gate-tooling
fetch-depth: 1
persist-credentials: false
token: ${{ secrets.GH_PAT_DON_PETRY || secrets.GH_PAT_WORKFLOWS }}
Comment on lines +105 to +110

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The workflow executes shell code from mutable ref v1 with a PAT, so a retagged or compromised source can run arbitrary commands with dispatch credentials. [security]

Assessment: 🔴 Critical · 🔁 Occurrence: Rarely

Use CodeAnt Skill Fix in Cursor Fix in VSCode Claude

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** .github/workflows/initiative-driver.yml
**Line:** 105:110
**Comment:**
	*Security: The workflow executes shell code from mutable ref `v1` with a PAT, so a retagged or compromised source can run arbitrary commands with dispatch credentials.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

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.

Skipping this one in this PR — routing it to the source of truth instead.

Validation: the finding is technically right that ref: v1 is a moving tag, but this file is a verbatim synced caller stub (SOURCE OF TRUTH: petry-projects/.github/standards/workflows/initiative-driver.yml; header: "Copy this file verbatim" / "never by editing callers"). The v1 tag on petry-projects/.github is the org's intentional release channel for the agent-rate-limit gate tooling — the same first-party mutable-ref trust model the org deliberately accepts for every reusable-workflow channel ref in this repo (e.g. add-to-project-reusable.yml@add-to-project/v1-ring1, agent-shield-reusable.yml@agent-shield/v2-ring1, all annotated first-party channel ref). Pinning it to a commit SHA here would (a) freeze the tooling and defeat the central rollout/rollback mechanism, and (b) diverge this stub from source of truth so the next sync reverts it.

Existing mitigations on this step are deliberate: persist-credentials: false, read-only fetch-depth: 1 checkout, and continue-on-error: true (fail-open). If the org decides first-party checkout refs must be SHA-pinned, that policy has to land in petry-projects/.github/standards/workflows/initiative-driver.yml so all enrolled repos change together — not as a one-off edit to this caller. Not stamping an addressed-marker since no code change was made here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Customized review instruction saved!

Instruction:

Do not flag the intentional mutable first-party channel ref in this verbatim synced caller stub; security policy changes must be made in the declared source-of-truth workflow instead.

Applied to:

  • .github/workflows/initiative-driver.yml

💡 To manage or update this instruction, visit: CodeAnt AI Settings

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.

Skipping — concur with the maintainer note above; no code change.

ref: v1 on petry-projects/.github is a first-party mutable channel ref (the org's intentional release channel for the agent-rate-limit gate tooling), and this file is a verbatim synced caller stub whose behavior is owned by the declared source of truth. SHA-pinning it here is explicitly disallowed per AGENTS.md "Release channel tags & the mutable-ref exception" — it would freeze the tooling, defeat central rollout/rollback, and diverge this stub from source of truth (reverted on next sync). Deliberate mitigations already on the step: persist-credentials: false, read-only fetch-depth: 1, and fail-open continue-on-error: true. Any policy to SHA-pin first-party checkout refs must land in petry-projects/.github's source-of-truth workflow so all enrolled repos change together. No addressed-marker (no code change).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Customized review instruction saved!

Instruction:

Do not flag the mutable v1 release-channel ref for SHA pinning in this synced caller stub; pinning must be managed in the source-of-truth workflow.

Applied to:

  • .github/workflows/initiative-driver.yml

💡 To manage or update this instruction, visit: CodeAnt AI Settings

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Customized review instruction saved!

Instruction:

Do not flag the intentional use of the mutable v1 release-channel ref in this synced workflow stub; SHA-pin changes must be made in the source-of-truth workflow instead.

Applied to:

  • .github/workflows/initiative-driver.yml

💡 To manage or update this instruction, visit: CodeAnt AI Settings

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.

Skipping — no code change (this pass concurs with the existing disposition on this thread). Per the dev-lead guardrail, ref: v1 on petry-projects/.github in the Checkout agent-rate-limit gate tooling step is a first-party mutable channel ref (the org's intentional release channel for the agent-rate-limit gate tooling) and must never be SHA-pinned — first-party channel tag, intentional mutable ref per AGENTS.md. This file is also a verbatim synced caller stub whose behavior is owned by the declared source of truth (petry-projects/.github/standards/workflows/initiative-driver.yml), so a one-off SHA-pin here would diverge from source-of-truth and be reverted on next sync. Deliberate mitigations remain on the step: persist-credentials: false, read-only fetch-depth: 1, and fail-open continue-on-error: true. Any policy to SHA-pin first-party checkout refs must land in the source-of-truth workflow so all enrolled repos change together. No addressed-marker (no code change).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Customized review instruction saved!

Instruction:

Do not flag the intentional mutable v1 ref for the first-party petry-projects/.github agent-rate-limit gate tooling checkout; this workflow is a synced caller stub and must follow the source-of-truth release channel.

Applied to:

  • .github/workflows/initiative-driver.yml

💡 To manage or update this instruction, visit: CodeAnt AI Settings

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.

Skipping — no code change. ref: v1 on petry-projects/.github (initiative-driver.yml line 106) is a first-party mutable channel ref — the org's intentional release/rollback channel for the agent-rate-limit gate tooling. Per the dev-lead guardrail and AGENTS.md "Release channel tags & the mutable-ref exception", first-party channel tags must not be SHA-pinned: pinning here would freeze the tooling, defeat central rollout/rollback, and diverge this verbatim synced caller stub from its source of truth (reverted on next sync). Deliberate mitigations already on the step remain: persist-credentials: false, read-only fetch-depth: 1, and fail-open continue-on-error: true. Any policy to SHA-pin first-party checkout refs must land in petry-projects/.github's source-of-truth workflow so all enrolled repos change together. No addressed-marker (no code change).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Customized review instruction saved!

Instruction:

Do not flag mutable first-party release or rollback channel refs for SHA pinning in this workflow; these refs are intentionally used for centralized rollout and rollback.

Applied to:

  • .github/workflows/initiative-driver.yml

💡 To manage or update this instruction, visit: CodeAnt AI Settings


- name: Agent rate-limit admission gate (enforcing)
# initiative-driver is the ONE canary agent type that ENFORCES (pure bash,
# no model spend, the direct #443 target — AC #5). Reads run history with
# the PAT (independent of the `permissions:` block) and emits
# decision=allow|defer to $GITHUB_OUTPUT; a defer is honoured by the `if:`
# on the dispatch step below. Fail-safe: the step can never fail the job.
continue-on-error: true
id: arl_gate
env:
GH_TOKEN: ${{ secrets.GH_PAT_DON_PETRY || secrets.GH_PAT_WORKFLOWS }}
ARL_ACTOR: ${{ github.actor }}
ARL_TRACKING_REPO: ${{ github.repository }}
Comment on lines +118 to +123

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The admission gate runs inside the cancel-in-progress group, so a newer event can cancel it during evaluation and allow bursts to bypass serialized rate-limit decisions. [race condition]

Assessment: 🟠 Major · 🔁 Occurrence: Sometimes

Use CodeAnt Skill Fix in Cursor Fix in VSCode Claude

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** .github/workflows/initiative-driver.yml
**Line:** 118:123
**Comment:**
	*Race Condition: The admission gate runs inside the cancel-in-progress group, so a newer event can cancel it during evaluation and allow bursts to bypass serialized rate-limit decisions.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

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.

Skipping — on validation the flagged race does not exist by design.

The admission gate does not rely on the cancel-in-progress concurrency group for serialization. As documented in the header (#443/#402): the gate "serializes a close-event + schedule burst AHEAD of the cancel-in-progress concurrency group" by deriving its concurrency / cooldown / daily-budget / consecutive-failure counters from this stub's own run history (no state backend). The concurrency group is only there so a close-burst + schedule don't fan out duplicate dispatches.

Walking the scenario the comment describes: events A then B arrive; A's job starts and begins gate evaluation; B arrives and cancels A (cancel-in-progress); B's job runs the gate against the true run history and decides allow/defer. A was cancelled before its dispatch step, so it never dispatched. That means a cancellation produces fewer dispatches, not more — the conservative direction for a rate limiter. A cancel is a clean no-op (never a dispatch, never a job failure, per #640 AC #1/#2); a defer likewise only skips the dispatch step. There is no path where cancelling mid-evaluation lets a burst bypass a limit and dispatch extra work.

No code change: the current concurrency + history-based gate is the intended design, and this is a verbatim synced stub whose behavior is owned by the source of truth. Not stamping an addressed-marker.

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.

Skipping — concur with the maintainer note above; no code change.

On validation the flagged race does not exist by design. The admission gate does not serialize via the cancel-in-progress concurrency group; per the header (lines 30–38, #443/#402) it derives its concurrency/cooldown/daily-budget/consecutive-failure counters from this stub's own run history (no state backend). Walking the scenario: if event B cancels A mid-evaluation, A is cancelled BEFORE its dispatch step, so it never dispatches — a cancellation yields FEWER dispatches, not more, which is the conservative direction for a rate limiter. A cancel is a clean no-op (never a dispatch, never a job failure, #640 AC #1/#2); a defer likewise only skips the dispatch step. There is no path where cancelling mid-evaluation lets a burst bypass a limit. This is also a verbatim synced stub whose behavior is owned by the source of truth. No addressed-marker (no code change).

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.

Skipping — no code change. Concur with the maintainer note above; on validation the flagged race does not exist by design.

The admission gate (lines 112-137) does NOT serialize via the cancel-in-progress concurrency group. Per the header (lines 30-38, #443/#402) it derives its concurrency/cooldown/daily-budget/consecutive-failure counters from this stub's own run history (no state backend); the concurrency group only prevents a close-burst + schedule from fanning out duplicate dispatches. Walking the flagged scenario: if event B cancels A mid-evaluation, A is cancelled BEFORE its dispatch step (line 139), so it never dispatches — a cancellation yields FEWER dispatches, not more, the conservative direction for a rate limiter. A cancel is a clean no-op (never a dispatch, never a job failure, #640 AC #1/#2); a defer likewise only skips the dispatch step (line 144). There is no path where cancelling mid-evaluation lets a burst bypass a limit. This is also a verbatim synced stub whose behavior is owned by the source of truth. No addressed-marker (no code change).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Customized review instruction saved!

Instruction:

Do not flag cancellation during admission-gate evaluation as a rate-limit bypass in this workflow; cancellation occurs before dispatch and can only reduce dispatches.

Applied to:

  • .github/workflows/initiative-driver.yml

💡 To manage or update this instruction, visit: CodeAnt AI Settings

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.

Skipping — no code change. Concur with the maintainer notes above; on validation the flagged race does not exist by design.

The admission gate (.github/workflows/initiative-driver.yml lines 112-137) does NOT serialize via the cancel-in-progress concurrency group. Per the header (lines 30-38, #443/#402) it derives its concurrency/cooldown/daily-budget/consecutive-failure counters from this stub's own run history (no state backend); the concurrency group only prevents a close-burst + schedule from fanning out duplicate dispatches. Walking the flagged scenario: if event B cancels A mid-evaluation, A is cancelled BEFORE its dispatch step (line 139), so it never dispatches — a cancellation yields FEWER dispatches, not more, the conservative direction for a rate limiter. A cancel is a clean no-op (never a dispatch, never a job failure, #640 AC #1/#2); a defer likewise only skips the dispatch step (line 144). There is no path where cancelling mid-evaluation lets a burst bypass a limit. This is also a verbatim synced stub whose behavior is owned by the source of truth. No addressed-marker (no code change).

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.

Skipping — no code change. Independently validated against the file and concur with the maintainer notes above: the flagged race does not exist by design.

The admission gate does not serialize via the cancel-in-progress concurrency group. Per the header (lines 30-38, #443/#402) it derives its concurrency/cooldown/daily-budget/consecutive-failure counters from this stub's own run history (no state backend); the concurrency group (lines 65-70) only prevents a close-burst + schedule from fanning out duplicate dispatches. Walking the flagged scenario: if event B cancels A mid-evaluation, A is cancelled before its dispatch step (line 139), so it never dispatches — a cancellation yields fewer dispatches, not more, the conservative direction for a rate limiter. A cancel is a clean no-op (never a dispatch, never a job failure, #640 AC #1/#2); a defer likewise only skips the dispatch step via the if: guard (line 144). There is no path where cancelling mid-evaluation lets a burst bypass a limit. This is also a verbatim synced stub whose behavior is owned by the source of truth. No addressed-marker (no code change).

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.

Skipping — no code change (this pass concurs with the existing disposition on this thread). On validation the flagged race does not exist by design: the admission gate does not serialize via the cancel-in-progress concurrency group — per the header (lines 30-38, #443/#402) it derives its concurrency/cooldown/daily-budget/consecutive-failure counters from this stub's own run history (no state backend); the concurrency group only prevents a close-burst + schedule from fanning out duplicate dispatches. Walking the scenario: if event B cancels A mid-evaluation, A is cancelled BEFORE its dispatch step (line 139), so it never dispatches — a cancellation yields FEWER dispatches, not more, the conservative direction for a rate limiter. A cancel is a clean no-op (never a dispatch, never a job failure, #640 AC #1/#2); a defer likewise only skips the dispatch step (line 144). There is no path where cancelling mid-evaluation lets a burst bypass a limit. This is also a verbatim synced stub owned by the source of truth. No addressed-marker (no code change).

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.

Skipping — no code change; the flagged race does not exist by design. The admission gate (initiative-driver.yml lines 112–137) does not serialize via the cancel-in-progress concurrency group. Per the header (lines 30–38, #443/#402) it derives its concurrency/cooldown/daily-budget/consecutive-failure counters from this stub's own run history (no state backend); the concurrency group only prevents a close-burst + schedule from fanning out duplicate dispatches. Walking the flagged scenario: if event B cancels A mid-evaluation, A is cancelled before its dispatch step (line 139), so it never dispatches — a cancellation yields fewer dispatches, not more, the conservative direction for a rate limiter. A cancel is a clean no-op (never a dispatch, never a job failure, #640 AC #1/#2); a defer likewise only skips the dispatch step (line 144). There is no path where cancelling mid-evaluation lets a burst bypass a limit. This is also a verbatim synced stub whose behavior is owned by the source of truth. No addressed-marker (no code change).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Customized review instruction saved!

Instruction:

Do not flag cancellation during admission-gate evaluation as a rate-limit bypass in this workflow; a cancelled run never reaches dispatch, so cancellation can only reduce dispatches.

Applied to:

  • .github/workflows/initiative-driver.yml

💡 To manage or update this instruction, visit: CodeAnt AI Settings

# For issue events use the event's issue number; for schedule/workflow_dispatch
# fall back to a configured repo variable so argate_escalate can post the
# breaker marker and apply the needs-human-review label (both are skipped when
# tracking_issue is empty). Set INITIATIVE_DRIVER_TRACKING_ISSUE to the epic
# or operations issue number for this repo to enable escalation on non-issue runs.
ARL_TRACKING_ISSUE: ${{ github.event.issue.number || vars.INITIATIVE_DRIVER_TRACKING_ISSUE || '' }}
run: |
set -euo pipefail
bash .arl-gate-tooling/scripts/agent-rate-limit-gate.sh initiative-driver \
--mode enforce \
--workflow initiative-driver.yml \
--actor "$ARL_ACTOR" \
--tracking-repo "$ARL_TRACKING_REPO" \
--tracking-issue "$ARL_TRACKING_ISSUE" || true

- name: Dispatch central initiative-driver
# Skipped (clean no-op) ONLY on an explicit `defer` — never a cancel, never
# a job failure (#640, AC #1/#2). Fail-OPEN: an empty/missing decision (the
# gate step or its tooling checkout failed) still dispatches, because an
# outage of the gate must never stop the fleet (ADR §7 fail-safe direction).
if: steps.arl_gate.outputs.decision != 'defer'
env:
GH_TOKEN: ${{ secrets.GH_PAT_DON_PETRY || secrets.GH_PAT_WORKFLOWS }}
run: |
Expand Down
Loading