Skip to content

ci(#7555): notify users when agent launch is unauthorized - #8190

Open
ifireball wants to merge 21 commits into
fullsend-ai:mainfrom
ifireball:codex/7555-authorization-denial-notice
Open

ifireball wants to merge 21 commits into
fullsend-ai:mainfrom
ifireball:codex/7555-authorization-denial-notice

Conversation

@ifireball

@ifireball ifireball commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Notify a human once when an agent launch is blocked by authorization policy.
  • Cover legacy slash-command routing and CEL/binary dispatch with the same deduplication marker.
  • Use the existing fullsend issues post-comment CLI (backed by forge.Client) in the dedicated notification job; the dispatch binary emits metadata only so legacy and CEL paths share one serialized producer.
  • Suppress notices for unavailable legacy stages and automatic triggers, and key deduplication on the immutable GitHub actor ID while using the login only for the mention.
  • An active kill switch now cleanly skips the requested stage instead of failing the routing job, matching the fail-closed no-dispatch behavior without emitting a false authorization denial.
  • Add forge-backed tests, workflow assertions, and user documentation.
  • Reviewed docs/contributing/ci-workflows.md; kept comment write access only on the dedicated notification job and serialized notification producers.

Fixes #7555

Verification

  • make lint
  • make go-vet
  • Targeted authorization, dispatch, scaffold, and coverage tests
  • make go-test reached all packages; the only failure was the unrelated existing runtime test TestExecuteBehaviourOp_CheckoutBranchRealShellExistingRemote, whose expected synthetic commit subject is changed by this checkout's DCO commit hook.

Cryptographic commit signing was unavailable because the configured gitsign flow requires browser authentication in this headless environment; the commit includes the required DCO sign-off and agent attribution.

Add first-denial notices for legacy slash commands and CEL dispatch while preserving shared authorization behavior.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Notify users once when authorization blocks an agent launch

🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Post one explanatory comment per user and work item when authorization blocks an agent launch.
• Cover legacy slash commands and CEL dispatch with the same deduplication marker.
• Add forge-backed tests, workflow assertions, documentation, and required comment permissions.
Diagram

graph TD
  Event["GitHub event"] --> Route["Legacy route"] --> Notice["Notice job"] --> Comments[("Issue comments")]
  Event --> CLI["Dispatch CLI"] --> Gate["Authorization gate"] --> Helper["Comment helper"] --> Comments
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize notices in the workflow
  • ➕ One comment implementation and one concurrency policy for both dispatch paths.
  • ➖ The CLI would need to emit denial metadata for the workflow, and dispatch outside GitHub Actions would no longer post notices independently.

Recommendation: Keep the CLI's forge-backed notice path so dispatch remains useful outside this workflow, and use the shared marker to coordinate with legacy routing. A centralized job would reduce duplication but couple CLI notifications to workflow execution.

Files changed (8) +276 / -15

Enhancement (2) +88 / -9
authorization_comment.goAdd actor-specific denial comment helper +64/-0

Add actor-specific denial comment helper

• Lists issue comments and posts an explanation only when the user's marker is absent. Limits posting to valid human GitHub actors and work-item numbers.

internal/harnessdispatch/authorization_comment.go

core.goExpose authorization denial in dispatch results +24/-9

Expose authorization denial in dispatch results

• Adds a result that distinguishes authorization denial from kill-switch and no-match outcomes. Retains the existing Dispatch API as a wrapper returning execution refs.

internal/harnessdispatch/core.go

Bug fix (2) +89 / -4
reusable-dispatch.ymlNotify on denied legacy slash commands +78/-1

Notify on denied legacy slash commands

• Tracks applicable commands and authorization-check failures so denied human requests produce route outputs without notifying for unrelated comments. Adds a serialized job that checks for the shared marker before posting, plus issue-comment permissions for both dispatch paths.

.github/workflows/reusable-dispatch.yml

dispatch.goPost best-effort notices from CEL dispatch +11/-3

Post best-effort notices from CEL dispatch

• Consumes the new dispatch result and attempts a forge-backed comment when authorization is denied. Comment failures produce an Actions warning without changing the empty dispatch result.

internal/cli/dispatch.go

Tests (3) +98 / -1
authorization_comment_test.goTest denial comment creation and filtering +84/-0

Test denial comment creation and filtering

• Uses a fake forge client to verify one comment per actor, separate notices for different actors, and no posting for bots, non-GitHub events, or invalid logins.

internal/harnessdispatch/authorization_comment_test.go

core_test.goTest dispatch result validation +5/-0

Test dispatch result validation

• Adds a test confirming that DispatchResult rejects an empty configuration directory.

internal/harnessdispatch/core_test.go

workflow_call_alignment_test.goAssert workflow notice wiring +9/-1

Assert workflow notice wiring

• Checks for denial outputs, the shared marker, serialization, and write permissions in the reusable workflow. Adjusts a slash-command assertion to accommodate applicability tracking.

internal/scaffold/workflow_call_alignment_test.go

Documentation (1) +1 / -1
cel-triggers-reference.mdDocument first-denial notices +1/-1

Document first-denial notices

• Explains that a denied human launch receives one explanatory comment per user and work item, without duplicates on repeated attempts.

docs/guides/user/cel-triggers-reference.md

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:27 AM UTC · Completed 11:41 AM UTC

Commit: 98ab826 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $3.46

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Site preview

Preview: https://e6ba9435-site.fullsend-ai.workers.dev

Commit: 6060bd04e8ad2ca37b9dac4967585059aef14a75

@qodo-code-review

qodo-code-review Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Any comment or PR from outsiders gets a denial notice ✓ Resolved
Description
runDispatch posts AuthorizationDeniedComment whenever DispatchResult reports an authorization
denial, but the shared IsAuthorized gate sets that flag before checking whether any harness
trigger matches. On repositories with registered harnesses, an outside contributor can open, edit,
or close an issue or PR, synchronize a PR, or leave a plain comment and be told an agent launch was
blocked despite never requesting one.
Code

internal/cli/dispatch.go[R113-114]

+	if result.AuthorizationDenied {
+		if err := harnessdispatch.AuthorizationDeniedComment(ctx, client, event); err != nil {
Relevance

●●● Strong

Denial notification must follow a matched launch request; current gating occurs before trigger
matching and broadens user-visible behavior.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
IsAuthorized requires write access for GitHub transitions other than label-added events and bot
review events, while DispatchResult returns AuthorizationDenied: true before
ListTriggeredHarnesses or MatchHarnesses runs. The event loader accepts issue events,
pull-request opened/synchronize/closed/edited events, and every issue comment regardless of its
body; the dispatch job runs for these events when agents are registered. The CLI then posts the
denial comment without checking for a command or matching trigger, unlike the legacy path, which
reports a denial only when COMMAND_APPLICABLE is true.

internal/harnessdispatch/auth.go[21-37]
internal/harnessdispatch/core.go[80-90]
internal/harnessdispatch/input/ghaevent.go[118-129]
.github/workflows/reusable-dispatch.yml[1596-1607]
internal/harnessdispatch/core.go[81-94]
internal/harnessdispatch/auth.go[21-36]
internal/harnessdispatch/input/ghaevent.go[327-351]
internal/cli/dispatch.go[113-119]

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 CEL dispatch path treats every authorization rejection as a denied agent launch, including events that match no harness trigger, and posts an "agent launch was blocked" comment for them.

## Fix Focus Areas
- internal/harnessdispatch/core.go[80-106]
- internal/cli/dispatch.go[113-120]

## Recommended Fix
Distinguish a denied launch attempt from an event with no applicable trigger. When authorization fails, still load candidate harnesses and run `MatchHarnesses`; set `AuthorizationDenied` and notify only if at least one trigger matched. Alternatively, limit denial notifications to explicit launch transitions such as comment commands. Add tests for issue-opened, plain-comment, and PR-synchronize events from a user with role none, asserting that no denial comment is posted when no launch was requested.

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


2. Denied launches can get duplicate comments ✓ Resolved
Description
AuthorizationDeniedComment checks for an existing marker and creates a denial comment in separate
operations, while the authorization-notice job serializes only its own comments. When an
unauthorized /fs-* comment reaches both the notice job and CEL-backed binary dispatch before
either comment is visible, both paths can pass the marker check and post on the same work item;
concurrent binary dispatches can do the same.
Code

.github/workflows/reusable-dispatch.yml[R709-710]

+    concurrency:
+      group: fullsend-authorization-notice-${{ github.repository }}-${{ needs.route.outputs.authorization_number }}
Relevance

●● Moderate

Duplicate-detection races have been rejected when APIs lack atomicity, but this workflow adds an
incomplete serialization boundary.

PR-#816

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow concurrency group applies to authorization-notice, but harness-dispatch can run
independently on the same event when inputs.matrix == '' and has no matching concurrency group.
Both paths list comments, check for the same marker, and then create a comment without an atomic or
shared serialization step, so each can observe that the marker is absent.

Avoid duplicate authorization comments
.github/workflows/reusable-dispatch.yml[701-745]
.github/workflows/reusable-dispatch.yml[1733-1756]
internal/cli/dispatch.go[106-120]
internal/harnessdispatch/authorization_comment.go[33-48]
.github/workflows/reusable-dispatch.yml[701-746]
.github/workflows/reusable-dispatch.yml[1596-1607]
internal/harnessdispatch/authorization_comment.go[33-49]
.github/workflows/reusable-dispatch.yml[1596-1608]
.github/workflows/reusable-dispatch.yml[1733-1754]

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 workflow notice and CEL-backed binary dispatch can independently pass the same marker check and post denial comments for one actor and work item. Concurrent binary dispatches have the same risk.

## Fix Focus Areas
- .github/workflows/reusable-dispatch.yml[701-746]
- .github/workflows/reusable-dispatch.yml[1596-1607]
- internal/cli/dispatch.go[113-120]
- internal/harnessdispatch/authorization_comment.go[33-49]

## Recommended Fix
Use one serialized notification path per work item across workflow and binary dispatch, or a shared idempotency mechanism that covers concurrent deliveries. One option is to make a single path responsible for each event type, such as having the CEL binary skip issue-comment slash commands handled by the legacy router; another is to coordinate both jobs under the same work-item concurrency group so their check-and-create steps cannot overlap. Preserve binary posting for CEL-based dispatch and test concurrent delivery.

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



Remediation recommended

3. Commenters can silence denial notices ✓ Resolved
Description
Both notification paths accept a marker found anywhere in an existing comment body without verifying
that Fullsend posted that comment. A user can include their own marker in the denied slash-command
comment, or post another user's marker beforehand, causing the corresponding explanatory notice to
be skipped.
Code

.github/workflows/reusable-dispatch.yml[R736-739]

+          if grep -Fq "${MARKER}" <<<"${COMMENTS}"; then
+            echo "Authorization-denial comment already exists for ${ACTOR_LOGIN}"
+            exit 0
+          fi
Relevance

●●● Strong

Team accepts strengthening marker ownership and author verification to prevent spoofed sticky
comments.

PR-#383
PR-#7907

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow fetches all issue-comment bodies and searches them for the marker; the binary likewise
searches Body alone, although the forge comment model and GitHub implementation provide Author.
The triggering slash-command comment is itself among the issue comments that can be listed.

.github/workflows/reusable-dispatch.yml[731-745]
internal/harnessdispatch/authorization_comment.go[33-49]
internal/forge/forge.go[427-434]
internal/forge/github/github.go[3144-3180]

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

## Issue description
User-authored issue comments can impersonate the marker used to suppress authorization notices.
## Fix Focus Areas
- .github/workflows/reusable-dispatch.yml[731-745]
- internal/harnessdispatch/authorization_comment.go[33-41]
## Recommended Fix
Inspect comment authors as well as bodies in both paths, and count a marker only when it belongs to the identity used to post Fullsend notices.

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


4. JSON dispatch can post live comments ✓ Resolved
Description
runDispatch calls the live comment notifier for a denied event regardless of which input driver
loaded it. With GitHub credentials available, a supplied JSON event claiming a GitHub human actor
and issue can now cause an API comment even though the JSON driver only reads the supplied
normalized event.
Code

internal/cli/dispatch.go[R113-114]

+	if result.AuthorizationDenied {
+		if err := harnessdispatch.AuthorizationDeniedComment(ctx, client, event); err != nil {
Relevance

●●● Strong

A JSON driver should not trigger live forge mutations; this is a deterministic side-effect boundary
bug.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The CLI constructs an authenticated client before selecting the driver, loads JSON without replacing
its claimed source, and sends every denied result to the new notifier. The notifier accepts
GitHub-sourced human events and calls the forge's issue-comment creation method.

internal/cli/dispatch.go[69-119]
internal/harnessdispatch/input/json.go[10-30]
internal/harnessdispatch/authorization_comment.go[19-49]

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

## Issue description
JSON-based dispatch evaluation can unexpectedly create a comment on a live GitHub issue.
## Fix Focus Areas
- internal/cli/dispatch.go[69-119]
- internal/harnessdispatch/input/json.go[10-30]
## Recommended Fix
Invoke authorization-denial commenting only for the live GitHub event input driver, unless an explicit side-effect option is provided for JSON input.

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


5. Permission lookup failures send false denial notices ✓ Resolved
Description
mapGitHubWebhook and mapIssueCommentEvent leave the actor at RoleNone when
GetCollaboratorPermission fails, and DispatchResult treats that unknown permission as
AuthorizationDenied. On a transient API error or rate limit, runDispatch posts a denial comment
to a potentially authorized maintainer, although the legacy bash path suppresses notices when
AUTH_CHECK_FAILED=true.
Code

internal/harnessdispatch/core.go[84]

+		return Result{AuthorizationDenied: true}, nil
Relevance

●●● Strong

Authorization-check failures currently default to denial notices; accepted precedent specifically
flags this failure mode for correction.

PR-#3820

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Both permission-lookup sites log errors but leave the role at RoleNone, which fails the shared
authorization gate and sets AuthorizationDenied; the CLI branch then posts the comment. The legacy
shell route records an API authorization-check failure and excludes it from its notice condition.

internal/harnessdispatch/input/ghaevent.go[84-96]
internal/harnessdispatch/input/ghaevent.go[293-304]
.github/workflows/reusable-dispatch.yml[259-265]
.github/workflows/reusable-dispatch.yml[433-442]
internal/harnessdispatch/input/ghaevent.go[69-86]
internal/harnessdispatch/input/ghaevent.go[289-304]
internal/harnessdispatch/core.go[81-84]

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

## Issue description
When a collaborator-permission lookup fails, the CEL path treats an undetermined role as a confirmed denial and posts a comment to a user who may be authorized.

## Fix Focus Areas
- internal/harnessdispatch/input/ghaevent.go[69-96]
- internal/harnessdispatch/input/ghaevent.go[289-304]
- internal/harnessdispatch/core.go[80-85]
- internal/cli/dispatch.go[113-119]

## Recommended Fix
Propagate permission-lookup failure separately from a confirmed insufficient role, for example through a `RoleUnknown` value or event flag. Continue to deny dispatch when authorization cannot be determined, but do not set `AuthorizationDenied` for notification or post a denial comment in that case, matching the legacy `AUTH_CHECK_FAILED` guard.

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


View medium (2)
6. Triage users told they are blocked while triage runs ✓ Resolved
Description
runDispatch posts the CEL gate’s denial for triage-only users even when the legacy router
authorizes their /fs-triage or /fs-review command. When both dispatch jobs run on an event
without a matrix, the legacy agent can launch while harness-dispatch tells the user they are
unauthorized; triage-role users can also receive that notice on their first comment on any issue or
PR in a repo with CEL agents.
Code

internal/cli/dispatch.go[R113-119]

+	if result.AuthorizationDenied {
+		if err := harnessdispatch.AuthorizationDeniedComment(ctx, client, event); err != nil {
+			// Authorization remains denied even if the explanatory comment cannot
+			// be posted. Keep the dispatch matrix empty and surface the best-effort
+			// notification failure in the Actions log.
+			fmt.Fprintf(os.Stderr, "::warning::could not post authorization-denial comment: %v\n", err)
+		}
Relevance

●●● Strong

Permission lookup failures should not silently become denials; closely matching authorization error
handling was accepted.

PR-#3820

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The legacy bash router accepts triage permission for /fs-triage and /fs-review, but the binary
gate’s IsAuthorized check requires write permission for an issue comment. The route and
harness-dispatch jobs both run when inputs.matrix == '', and the CLI branch posts a comment on the
binary denial, allowing the same event to produce an authorized legacy launch and a CEL denial
notice.

.github/workflows/reusable-dispatch.yml[306-311]
internal/harnessdispatch/auth.go[36-36]
.github/workflows/reusable-dispatch.yml[104-107]
.github/workflows/reusable-dispatch.yml[1596-1598]
.github/workflows/reusable-dispatch.yml[303-325]
.github/workflows/reusable-dispatch.yml[1596-1608]
internal/harnessdispatch/auth.go[21-36]
internal/cli/dispatch.go[106-119]

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

## Issue description
Triage-role users can launch legacy triage or review agents while the parallel CEL dispatch path tells them they are unauthorized. The notice can also appear on a triage-role user’s first issue or PR comment in a repo with CEL agents.

## Fix Focus Areas
- internal/cli/dispatch.go[113-120]
- internal/harnessdispatch/core.go[80-85]
- internal/harnessdispatch/auth.go[21-36]
- .github/workflows/reusable-dispatch.yml[303-325]

## Recommended Fix
Coordinate notification decisions across the two dispatch paths so the CEL denial is emitted only when a CEL harness trigger matched the event and the legacy router has not authorized it. For issue-comment slash commands handled by the legacy route, skip the CEL notice or make authorization respect their triage-level threshold; do not publish a denial when the legacy route has authorized the launch.

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


7. Workflow edit omits guide attestation ✗ Dismissed
Description
The new authorization-notice job uses github.token with issue-comment write permission, but
neither the workflow nor the PR description says that docs/contributing/ci-workflows.md was
consulted. This omission applies to a workflow change that adds a token-bearing write job and
expands permissions in two existing jobs.
Code

.github/workflows/reusable-dispatch.yml[R707-713]

+    permissions:
+      issues: write
+    concurrency:
+      group: fullsend-authorization-notice-${{ github.repository }}-${{ needs.route.outputs.authorization_number }}
+      cancel-in-progress: false
+    env:
+      GH_TOKEN: ${{ github.token }}
Evidence
Rule 3201588 requires explicit evidence of consulting the CI workflow guide for workflow and
permission changes. The diff adds a job with issues: write and github.token, while the supplied
PR description contains no guide reference and the workflow contains no such comment.

Rule 3201588: Consult CI workflow guide when modifying GitHub Actions workflows or secrets
.github/workflows/reusable-dispatch.yml[701-716]
.github/workflows/reusable-dispatch.yml[107-111]
.github/workflows/reusable-dispatch.yml[1600-1604]

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 adds workflow token permissions without the explicit CI-guide consultation required for such changes.

## Fix Focus Areas
- .github/workflows/reusable-dispatch.yml[701-713]

## Recommended Fix
Review docs/contributing/ci-workflows.md against the changed jobs and state in the PR description that the guide was consulted, noting any resulting adjustments.

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



Informational

8. Routing job gets issue write access it never uses ✓ Resolved
Description
The route job's permission is raised from issues: read to issues: write, but it only writes
step outputs; the separate authorization-notice job posts the comment with its own issues: write
token. That widens the token of a job that processes untrusted comment bodies and calls gh, and
the new scaffold test assertion locks the extra privilege in.
Code

.github/workflows/reusable-dispatch.yml[110]

+      issues: write
Relevance

●●● Strong

Unnecessary write scope on untrusted workflow jobs is a clear security fix; permission hardening
precedents were accepted.

PR-#1688
PR-#5502

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The route step only echoes authorization_* values to GITHUB_OUTPUT. The comment is created in
the authorization-notice job, which declares its own issues: write.

.github/workflows/reusable-dispatch.yml[433-442]
.github/workflows/reusable-dispatch.yml[701-746]

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 route job has issues: write even though only the authorization-notice job posts comments.

## Fix Focus Areas
- .github/workflows/reusable-dispatch.yml[108-111]
- internal/scaffold/workflow_call_alignment_test.go[796-797]

## Recommended Fix
Set `issues: read` on the route job again and keep `issues: write` only on the authorization-notice and harness-dispatch jobs. If needed, change the test so it checks the notice job's permission specifically.

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


Grey Divider

Context sources
✅ Compliance rules (platform): 89 rules
✅ Skills: 5 invoked
  writing-user-docs
  writing-adrs
  check-patch-coverage
  cutting-releases
  adr-corner
✅ Cross-repo context — repo relationships
  Explored: repo: fullsend-ai/experiments (sha: 3f1614fa) — View relationship
  Explored: repo: fullsend-ai/.fullsend (sha: 7c163cad) — View relationship
  Explored: repo: fullsend-ai/metrics (sha: aad23f91) — View relationship
  Explored: repo: fullsend-ai/pi-xai-vertex (sha: 8f50bdda) — View relationship
  Explored: repo: fullsend-ai/scribe (sha: 0f3eb528) — View relationship
  Explored: repo: fullsend-ai/agents (sha: f3dd8897) — View relationship
  Explored: repo: fullsend-ai/pi-anthropic-vertex (sha: 960eff35) — View relationship
  Explored: repo: fullsend-ai/mlflow-assessments (sha: 2fdb0120) — View relationship
Review mode: Auto: 🧠 Deep: Authorization behavior spans CI, dispatch, permissions, deduplication, and multiple independent paths.

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/workflows/reusable-dispatch.yml Outdated
Comment thread .github/workflows/reusable-dispatch.yml
Comment thread .github/workflows/reusable-dispatch.yml Outdated
Comment thread internal/cli/dispatch.go Outdated
Comment thread internal/cli/dispatch.go Outdated
Comment thread internal/harnessdispatch/core.go Outdated
Comment thread internal/cli/dispatch.go Outdated
Comment thread .github/workflows/reusable-dispatch.yml Outdated
@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Oct 8, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

<!-- fullsend:risk-assessment -->
Risk Assessment: elevated (3/5)

Details

Large changeset modifying protected CI workflow permissions and core dispatch logic across frequently churned files without a feature flag, balanced by complete issue acceptance criteria coverage.

Previous run

<!-- fullsend:risk-assessment -->
Risk Assessment: elevated (3/5)

Details

Large changeset modifying protected CI workflow permissions and core dispatch logic across frequently churned files without a feature flag, balanced by complete issue acceptance criteria coverage.

Previous run (2)

<!-- fullsend:risk-assessment -->
Risk Assessment: elevated (3/5)

Details

Large changeset modifying protected CI workflow permissions and core dispatch logic across frequently churned files without a feature flag, balanced by complete issue acceptance criteria coverage.

Previous run (3)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Protected CI workflow changes and high historical churn in dispatch files are balanced by solid test coverage, additive revertibility, and exact scope alignment with the linked issue.

Previous run (4)

<!-- fullsend:risk-assessment -->
Risk Assessment: moderate (2/5)

Details

Protected CI workflow changes and high historical churn in dispatch files are balanced by moderate diff size, solid test coverage, and exact scope alignment with the linked issue.

Previous run (5)

Risk Assessment: moderate (2/5)

Details

Protected CI workflow changes and high historical churn in dispatch files are balanced by moderate diff size, solid test coverage, and exact scope alignment with the linked issue.

Previous run (6)

Risk Assessment: moderate (2/5)

Details

Protected CI workflow modifications and elevated historical churn in dispatch components are balanced by solid test coverage, moderate change size, and exact scope alignment with the linked issue.

Previous run (7)

Risk Assessment: moderate (2/5)

Details

Protected CI workflow modifications and elevated git churn across dispatch files are balanced by strong test coverage and exact scope alignment with linked issue #7555.

Previous run (8)

Risk Assessment: moderate (2/5)

Details

Moderate diff size with protected CI workflow changes and elevated workflow churn is balanced by solid test coverage, complete acceptance criteria alignment, and established author history.

Previous run (9)

Risk Assessment: moderate (2/5)

Details

Moderate diff size with protected CI workflow changes and elevated workflow churn is balanced by solid test coverage, complete acceptance criteria alignment, and established author history.

Previous run (10)

Risk Assessment: moderate (2/5)

Details

Moderate diff size with protected CI workflow changes and elevated workflow churn is balanced by solid test coverage, complete acceptance criteria alignment, and established author history.

Previous run (11)

Risk Assessment: moderate (2/5)

Details

Moderate risk driven by CI workflow modifications and high git churn across dispatch workflows, balanced by clear issue alignment, solid test coverage, and a focused blast radius.

Previous run (12)

Risk Assessment: moderate (2/5)

Details

Moderate-sized change modifying CI workflow dispatch and core dispatch notification paths without a feature flag, counterbalanced by good test coverage, low author contention across codebase files, and exact alignment with linked issue #7555.

Previous run (13)

Risk Assessment: moderate (2/5)

Details

Moderate-sized change modifying CI workflow dispatch and core dispatch notification paths without a feature flag, counterbalanced by good test coverage, low author contention across codebase files, and exact alignment with linked issue #7555.

Previous run (14)

Risk Assessment: moderate (2/5)

Details

Protected CI workflow changes and high historical churn in dispatch files are balanced by moderate diff size, solid test coverage, and exact scope alignment with the linked issue.

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review

Findings

Medium

  • [logic-error] .github/workflows/reusable-dispatch.yml:649 — The "Resolve PR head for issue_comment events" step still runs after the kill switch skips dispatch. Its if: checks steps.route.outputs.stage != '' plus the role-check/agent-check skipped outputs, but not steps.kill-switch.outputs.skipped. Those two checks are themselves gated on the kill switch, so their outputs are empty when it fires, and the route step's own stage output stays non-empty (only the job-level stage output applies the guard). An authorized /fs-fix on a fork PR therefore fails the route job at the fork guard (exit 1) while dispatch is paused, and a PR API failure for fix/review stages fails it the same way. This contradicts the new successful-skip behavior.
    Remediation: Add steps.kill-switch.outputs.skipped != 'true' to this step's if: and to any other post-routing preparation steps keyed on steps.route.outputs.stage. Add a test showing a paused /fs-fix request on a fork PR skips PR resolution and leaves routing successful.

  • [edge-case] internal/harnessdispatch/core.go:148 — Per-harness denial metadata ignores AuthorizationAvailable(). When the live collaborator-permission lookup fails but an OWNERS file upgrades the effective role to triage, the event-level gate passes and the availability short-circuit is skipped. A matching harness with a higher role threshold then sets AuthorizationDenied=true, so the actor gets a "not authorized" comment even though their real permissions were never resolved and could include write access. Execution stays fail-closed; the defect is a false denial notice.
    Remediation: Suppress the per-harness authorizationDenied result when opts.Event.AuthorizationAvailable() is false, while keeping execution that OWNERS grants independently allow. Add a test combining MarkAuthorizationUnavailable(), an OWNERS reviewer entry, and a matching write-threshold slash-command harness.

  • [incorrect-doc] docs/normative/authorization/v1/README.md:438 — Under "CEL triggers: routing only", the document says an event that fails authorization never reaches CEL evaluation, and that FilterAuthorizedHarnesses() removes unauthorized harnesses before MatchHarnesses() runs. DispatchResult in internal/harnessdispatch/core.go now runs MatchHarnesses() as a non-dispatching probe for unauthorized events and denied candidates, and no longer calls FilterAuthorizedHarnesses() there. Execution still projects no refs for denied harnesses, but the normative text no longer matches the code. This file is not part of the PR diff, so it was not updated.
    Remediation: Update the paragraph (around lines 436-446) to say the execution gate remains fail-closed (CEL can never weaken authorization and unauthorized harnesses never project refs), and that event-backed dispatch evaluates candidate triggers in a non-dispatching probe solely to tell genuine launch attempts apart from non-matching events.

  • [protected-path] .github/workflows/reusable-dispatch.yml — This PR modifies a protected path. It adds the authorization-notice job with issues: write, changes route outputs, and changes kill-switch behavior. The PR links issue dispatch: notify users when agent launch is blocked by authorization policy #7555 and explains the rationale, but changes to governance and infrastructure files always require human approval, regardless of context.

Low

  • [tier-mismatch] .github/workflows/reusable-dispatch.yml — The PR adds a new user-facing capability (comments when a launch is blocked by authorization policy or paused by the kill switch; the linked issue is a feature request) but its title and commit subjects use ci(#7555) / fix(#7555). Per COMMITS.md, ci is excluded from release notes while feat marks new user-facing functionality.
    Remediation: Retitle the PR and the relevant commit subjects with a feat(...) prefix.

  • [scope-creep] .github/workflows/reusable-dispatch.yml:543 — Issue dispatch: notify users when agent launch is blocked by authorization policy #7555 covers notifying users when authorization policy blocks a launch. The PR also changes kill-switch behavior from failing the routing job to a quiet skip, adds a separate paused-dispatch notice, makes internal/harnessdispatch/core.go evaluate triggers while the kill switch is active, and amends docs/reference/config-reference.md. The PR body discloses this, but the linked issue does not authorize it.
    Remediation: Descope the kill-switch pause behavior, or file and link an issue that authorizes it.

  • [breaking-schema] docs/normative/normalized-event/v1/normalized-event.schema.json:304 — The new optional actor.numeric_id is additive, but the actor object sets additionalProperties: false, so consumers validating against a pinned copy of the v1 schema will reject events that carry the field. The README documents it as an additive v1.x change and warns pinned-copy consumers.
    Remediation: Keep the README note; consider a schema changelog entry.

  • [backward-incompatible] .github/workflows/reusable-dispatch.yml:543 — An active kill switch now makes the route job succeed with an empty stage output; before this change the job failed. Callers that treated route-job success as "dispatch proceeded" must now gate on stage != ''. The change is documented in ci-workflows.md and config-reference.md.
    Remediation: Call out the change in release notes and confirm the shim and scaffold workflows gate on the stage output.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl8xNTg1NzA0MGJjYzk5NDQ1IiwibGluZSI6NjQ5LCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJpbmNvcnJlY3QtZG9jIiwiZmlsZSI6ImRvY3Mvbm9ybWF0aXZlL2F1dGhvcml6YXRpb24vdjEvUkVBRE1FLm1kIiwiaWQiOiJmX2E0MzI4MDk2MDAwMzVmY2MiLCJsaW5lIjo0MjQsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InNjb3BlLWNyZWVwIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl9kYjQ1NjM2NDQ0NTVjMjhhIiwibGluZSI6NTQzLCJhY3Rpb25hYmxlIjpmYWxzZX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoidGllci1taXNtYXRjaCIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfZDBkYmJkOTgyODQzYjIzNSIsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJicmVha2luZy1zY2hlbWEiLCJmaWxlIjoiZG9jcy9ub3JtYXRpdmUvbm9ybWFsaXplZC1ldmVudC92MS9ub3JtYWxpemVkLWV2ZW50LnNjaGVtYS5qc29uIiwiaWQiOiJmXzUxZmUzYWU3ZGZkYWYzNmIiLCJsaW5lIjozMDQsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJiYWNrd2FyZC1pbmNvbXBhdGlibGUiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzUzZTBlNzI0ZDAxOTEyYjEiLCJsaW5lIjo1NDMsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoiaW5mbyIsImNhdGVnb3J5IjoicGVybWlzc2lvbi1leHBhbnNpb24iLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzZhMWRmMmNhZjE3NmE4MTciLCJsaW5lIjo3ODYsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoiaW5mbyIsImNhdGVnb3J5IjoiY29tcGxleGl0eS1yYXRpbyIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfZDViMzE1MDY1MTFmMmYwMCIsImxpbmUiOjc3MiwiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl9mYzQ2ZmQ2YmY3NWY4MDg5IiwibGluZSI6NDgyLCJhY3Rpb25hYmxlIjp0cnVlfV0sImRpc3Bvc2l0aW9ucyI6W3siaWQiOiJmX2ZjNDZmZDZiZjc1ZjgwODkiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl8xNTg1NzA0MGJjYzk5NDQ1Iiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmX2RiNDU2MzY0NDQ1NWMyOGEiLCJzdGF0dXMiOiJyZWNsYXNzaWZpZWQifSx7ImlkIjoiZl82YTFkZjJjYWYxNzZhODE3Iiwic3RhdHVzIjoicmVjbGFzc2lmaWVkIn0seyJpZCI6ImZfZDViMzE1MDY1MTFmMmYwMCIsInN0YXR1cyI6InJlY2xhc3NpZmllZCJ9LHsiaWQiOiJmX2QwZGJiZDk4Mjg0M2IyMzUiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfNTFmZTNhZTdkZmRhZjM2YiIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl81M2UwZTcyNGQwMTkxMmIxIiwic3RhdHVzIjoib3BlbiJ9XX0= -->

Review

Findings

Medium

  • [logic-error] .github/workflows/reusable-dispatch.yml:649 — The Resolve PR head for issue_comment events step still runs when the kill switch is active. Its condition checks steps.route.outputs.stage (still set for authorized slash commands) but not steps.kill-switch.outputs.skipped, unlike the neighbouring Validate agents config, role-check and agent-check steps. /fs-fix on a fork PR, or a failed PR API request for /fs-fix//fs-review, still fails the route job even though dispatch is meant to be quietly paused. Before this PR the kill switch exited with exit 1 ahead of these steps.
    Remediation: Add steps.kill-switch.outputs.skipped != 'true' to the step's if: and add a test that paused PR commands skip PR resolution (fork PRs and API failures).
  • [incorrect-doc] docs/normative/authorization/v1/README.md:424 — The Authorization Contract says an event that fails authorization "never reaches CEL evaluation" and that IsAuthorized() runs before MatchHarnesses(). After this PR, internal/harnessdispatch/core.go runs MatchHarnesses for events that fail authorization (to set AuthorizationDenied), and with the kill switch active it runs before authorization is evaluated. Unauthorized events still yield no execution refs, but the normative wording is now wrong. (File is not in the PR diff.)
    Remediation: Update the passage to say unauthorized events (and, under the kill switch, all events) may still undergo CEL matching solely to decide whether to post a denial/pause notice, while a CEL match can never grant execution or weaken the authorization gate.
  • [protected-path] .github/workflows/reusable-dispatch.yml — This PR modifies a protected path (.github/). It links dispatch: notify users when agent launch is blocked by authorization policy #7555 and explains the rationale, but human approval is always required for protected-path changes, regardless of context.

Low

  • [scope-creep] .github/workflows/reusable-dispatch.yml:543 — Kill-switch handling changes from exit 1 to a silent skip with pause notices; dispatch: notify users when agent launch is blocked by authorization policy #7555 only covers authorization-policy denials. Mitigated by the change being tied to the shared notification job.
  • [tier-mismatch] .github/workflows/reusable-dispatch.yml — The ci prefix denotes CI/CD changes omitted from release notes, but the PR adds user-visible behaviour, Go dispatch changes and a normalized-event field; it belongs under feat/fix.
  • [breaking-schema] docs/normative/normalized-event/v1/normalized-event.schema.json:304 — Optional actor.numeric_id is added to a v1 schema with additionalProperties: false without a version bump; strict pinned validators will reject events carrying it.
    Remediation: Document it as additive/backward-compatible for producers only.
  • [backward-incompatible] .github/workflows/reusable-dispatch.yml:543 — An active kill switch now yields a successful skipped run instead of a failure; adopter alerting or required checks that relied on the failure will change behaviour.
    Remediation: Mention in release notes, with a ! marker if judged breaking.

Earlier findings

  • .github/workflows/reusable-dispatch.yml:482, logic-error — resolved: the reintroduced Check for existing PRs step and its PR_SKIPPED plumbing were removed.
  • .github/workflows/reusable-dispatch.yml:592, scope-creep — reclassified medium → low: kill-switch pause handling is tied to the shared notification job and distinguishes pause from denial.
  • .github/workflows/reusable-dispatch.yml:837, permission-expansion — reclassified to info: minimal, justified, isolated grant.
  • .github/workflows/reusable-dispatch.yml:823, complexity-ratio — reclassified to info: the CLI install/cache steps are required by the forge-abstraction rule and mirror existing jobs.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (2)

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl9mYzQ2ZmQ2YmY3NWY4MDg5IiwibGluZSI6NDgyLCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfMTU4NTcwNDBiY2M5OTQ0NSIsImxpbmUiOjY5OCwiYWN0aW9uYWJsZSI6dHJ1ZX0seyJzZXZlcml0eSI6Im1lZGl1bSIsImNhdGVnb3J5Ijoic2NvcGUtY3JlZXAiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmX2RiNDU2MzY0NDQ1NWMyOGEiLCJsaW5lIjo1OTIsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJwZXJtaXNzaW9uLWV4cGFuc2lvbiIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfNmExZGYyY2FmMTc2YTgxNyIsImxpbmUiOjgzNywiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImNvbXBsZXhpdHktcmF0aW8iLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmX2Q1YjMxNTA2NTExZjJmMDAiLCJsaW5lIjo4MjMsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJ0aWVyLW1pc21hdGNoIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl9kMGRiYmQ5ODI4NDNiMjM1IiwiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImJyZWFraW5nLXNjaGVtYSIsImZpbGUiOiJkb2NzL25vcm1hdGl2ZS9ub3JtYWxpemVkLWV2ZW50L3YxL25vcm1hbGl6ZWQtZXZlbnQuc2NoZW1hLmpzb24iLCJpZCI6ImZfNTFmZTNhZTdkZmRhZjM2YiIsImxpbmUiOjMwNCwiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImJhY2t3YXJkLWluY29tcGF0aWJsZSIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfNTNlMGU3MjRkMDE5MTJiMSIsImxpbmUiOjU5MCwiYWN0aW9uYWJsZSI6ZmFsc2V9XX0= -->

Review

Findings

Medium

  • [logic-error] .github/workflows/reusable-dispatch.yml:482 — The PR re-adds the pr-check ("Check for existing PRs") gate that main removed in refactor(workflow)!: remove existing-pr gate on the workflow #8133 and that dispatch: notify users when agent launch is blocked by authorization policy #7555 does not ask for. It clears the routed stage, so an authorized /fs-code --force is blocked whenever another author's open PR closes the issue. Its ::warning:: also builds text from gh stderr with incomplete workflow-command sanitization.
    Remediation: Rebase on main and remove the pr-check step and all steps.pr-check.outputs.skipped references (stage output, agent-check if:, PR_SKIPPED, related tests).
  • [logic-error] .github/workflows/reusable-dispatch.yml:698 — The kill switch now sets skipped=true and exits 0, but the PR-head resolution step still gates on steps.route.outputs.stage without checking the kill-switch skipped output. A paused /fs-fix on a fork PR, or a paused /fs-review with a failing PR lookup, still fails the route job.
    Remediation: Add steps.kill-switch.outputs.skipped != 'true' to the PR-head resolution and event-payload steps (or use one effective-stage output) and add a scaffold test for the paused fork-PR path.
  • [scope-creep] .github/workflows/reusable-dispatch.yml:592 — dispatch: notify users when agent launch is blocked by authorization policy #7555 asks only for a first-denial notice; the PR also changes the kill-switch contract (route no longer fails, separate pause notice, updated config reference). It is disclosed in the PR description, but needs maintainer sign-off or a separate PR.
  • [protected-path] .github/workflows/reusable-dispatch.yml — Modifies a protected path (new job with issues: write, kill-switch and routing changes). The PR links dispatch: notify users when agent launch is blocked by authorization policy #7555 and gives a rationale, but human approval is always required.

Low

  • [permission-expansion] .github/workflows/reusable-dispatch.yml:837 — New authorization-notice job gets issues: write; justified by dispatch: notify users when agent launch is blocked by authorization policy #7555 and confined to the comment producer. Noted for human confirmation.
  • [complexity-ratio] .github/workflows/reusable-dispatch.yml:823 — About +356 lines and many new cross-job outputs; part of it comes from the out-of-scope kill-switch and existing-PR branches.
  • [tier-mismatch] .github/workflows/reusable-dispatch.yml — ci(#7555) is excluded from release notes, but the PR adds user-visible behavior and Go production changes; a feat/fix prefix fits better.
  • [breaking-schema] docs/normative/normalized-event/v1/normalized-event.schema.json:304 — Optional actor.numeric_id added to v1 where actor has additionalProperties: false; older v1 validators will reject events carrying it. Add a compatibility note.
  • [backward-incompatible] .github/workflows/reusable-dispatch.yml:590 — An active kill switch no longer fails the route job; callers must key off the stage output. Note it in release notes.
  • [sub-agent-failure] N/A — The style-conventions sub-agent timed out twice, so style and convention checks were not performed.
  • [sub-agent-failure] N/A — The challenger did not run (time budget); the pre-challenger finding set is used.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (3)

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl8yYWYwOTE1MzQ4Zjc1MzI3IiwibGluZSI6Njk4LCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiJpbnRlcm5hbC9oYXJuZXNzZGlzcGF0Y2gvY29yZS5nbyIsImlkIjoiZl8xMzQxYzdkYWU4OGE3ZjYzIiwibGluZSI6NjEsImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImJyZWFraW5nLWNvbmZpZyIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfYzZiNTZmMmE1ZjdjMDA4MiIsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJtaXNsZWFkaW5nLWxhYmVsIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl9kMjJlZjYzNjRjMDhlYjRiIiwiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6Im5hbWluZy1jb252ZW50aW9uIiwiZmlsZSI6ImludGVybmFsL2NsaS9kaXNwYXRjaF90ZXN0LmdvIiwiaWQiOiJmXzU0ZWYyYTkwODI0MmIxZDQiLCJsaW5lIjoyMCwiYWN0aW9uYWJsZSI6dHJ1ZX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoiYnJlYWtpbmctc2NoZW1hIiwiZmlsZSI6ImRvY3Mvbm9ybWF0aXZlL25vcm1hbGl6ZWQtZXZlbnQvdjEvbm9ybWFsaXplZC1ldmVudC5zY2hlbWEuanNvbiIsImlkIjoiZl9iZGY3ZGI0ZWQ5OGM0ZjMxIiwibGluZSI6MzA0LCJhY3Rpb25hYmxlIjpmYWxzZX1dfQ== -->

Review

Findings

Medium

  • [logic-error] .github/workflows/reusable-dispatch.yml:698 — The PR-head resolution step does not check steps.kill-switch.outputs.skipped. When the kill switch is active, role-check and agent-check are skipped, so their skipped outputs are empty and the != 'true' guards pass while steps.route.outputs.stage stays populated. The step can still exit 1 (fork guard for /fs-fix on a fork PR, or a failed PR lookup), so an engaged kill switch can still fail the routing job instead of the promised quiet skip. Validate routed stage and Build event payload share the gap.
    Remediation: Add steps.kill-switch.outputs.skipped != 'true' to PR-head resolution and the other post-routing steps gated on steps.route.outputs.stage; add regression tests for an active kill switch with /fs-fix on a fork PR and with a failed PR lookup.
  • [logic-error] internal/harnessdispatch/core.go:61 — DispatchResult returns kill_switch before deciding whether any harness matches the event, and writeDispatchOutput emits pause metadata for any human slash comment. With agents registered and the kill switch on, an unmatched /fs-<anything> comment, including the supported /fs-fix-stop (handled separately by stop-fix), produces a "dispatch is paused / request not run" comment even though no agent launch was requested. The authorization-denied branch requires len(matched) > 0, so the asymmetry looks unintended.
    Remediation: Only emit pause-notification metadata when a harness actually matched the launch request (while keeping all execution blocked); add tests for unmatched commands and /fs-fix-stop alongside matching custom launch commands.
  • [breaking-config] .github/workflows/reusable-dispatch.yml — An active kill switch used to fail the route job (exit 1); it now succeeds with an empty stage output and runs a downstream job that posts comments (fullsend:dispatch-paused). Callers or branch-protection checks that treated that failure as the halt signal will see a green run. This is documented but not flagged as breaking (COMMITS.md requires !), and goes beyond what dispatch: notify users when agent launch is blocked by authorization policy #7555 asks for (a notice on unauthorized launches). Impact is limited in the base shim template, which has no downstream jobs reading route failure.
    Remediation: Split the kill-switch quiet-skip and pause-notice into a separately tracked change, or call it out in release notes and mark with ! if downstream callers depend on the failure.
  • [protected-path] .github/workflows/reusable-dispatch.yml — This PR modifies a protected path. The PR links issue dispatch: notify users when agent launch is blocked by authorization policy #7555 and explains the rationale, but human approval is always required for protected-path changes regardless of context.

Low

  • [misleading-label] .github/workflows/reusable-dispatch.yml — The ci title prefix marks the change as internal CI maintenance (excluded from release notes), but the PR adds user-visible behavior (issue comments) and a Go/schema field, which COMMITS.md maps to feat (judgment call).
    Remediation: Consider retitling to feat(#7555): notify users when agent launch is unauthorized.
  • [naming-convention] internal/cli/dispatch_test.go:20 — The new writeDispatchOutput tests deviate from the Test<Target>_<Scenario> convention used in this file; seven of eight use the TestWriteAuthorizationOutput prefix rather than the function name.
    Remediation: Rename to TestWriteDispatchOutput_<Scenario>.
  • [breaking-schema] docs/normative/normalized-event/v1/normalized-event.schema.json:304 — The optional actor.numeric_id is added to a schema whose actor object sets additionalProperties: false, so consumers with pinned strict schema copies will reject payloads containing it. Additive, optional and documented in the README; advisory.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (4)

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl8xMzhkODVmMTZjZjlmMDI0IiwibGluZSI6NzcxLCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJ0ZXN0LWluYWRlcXVhdGUiLCJmaWxlIjoiaW50ZXJuYWwvc2NhZmZvbGQvd29ya2Zsb3dfY2FsbF9hbGlnbm1lbnRfdGVzdC5nbyIsImlkIjoiZl9lZDFhYmQ5NDQxZWY1ZDE5IiwibGluZSI6MTUxLCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfZDlkMWQzY2ZmY2I1YTdhNiIsImxpbmUiOjY5OCwiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImluY29tcGxldGUtZG9jIiwiZmlsZSI6ImRvY3MvZ3VpZGVzL3VzZXIvY3VzdG9taXppbmctYWdlbnRzLm1kIiwiaWQiOiJmX2ExYWRiNWE0NWViMTgxZTAiLCJsaW5lIjo1ODksImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InN0YWxlLWRvYyIsImZpbGUiOiJkb2NzL25vcm1hdGl2ZS9hdXRob3JpemF0aW9uL3YxL1JFQURNRS5tZCIsImlkIjoiZl8yMDA2OTI3OTdjMjhlMmE0IiwibGluZSI6NDA5LCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJmYWlsLW9wZW4iLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmX2Y5NDBiMThlYjRjODg1OGMiLCJsaW5lIjo2ODQsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJmYWlsLW9wZW4iLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzUxMDNiODBlMmVkODc5OTQiLCJsaW5lIjo2NTYsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJtaXNsZWFkaW5nLWxhYmVsIiwiZmlsZSI6bnVsbCwiaWQiOiJmXzZlZWRiYmRlODE5ZDgzN2MiLCJhY3Rpb25hYmxlIjpmYWxzZX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5Ijoic2NvcGUtY3JlZXAiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzk1YTY0ODlhOWE1YjE1NTkiLCJsaW5lIjo1OTgsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJkZXNpZ24tZGlyZWN0aW9uIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl9hYjFiOTRiOTMwNmFhZmVlIiwibGluZSI6ODE1LCJhY3Rpb25hYmxlIjpmYWxzZX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoiY29kZS1vcmdhbml6YXRpb24iLCJmaWxlIjoiaW50ZXJuYWwvY2xpL2Rpc3BhdGNoLmdvIiwiaWQiOiJmXzA1MTMwMTBlYjM0YmVlODEiLCJsaW5lIjoxNDQsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJiYWNrd2FyZC1pbmNvbXBhdGlibGUiLCJmaWxlIjoiZG9jcy9ub3JtYXRpdmUvbm9ybWFsaXplZC1ldmVudC92MS9ub3JtYWxpemVkLWV2ZW50LnNjaGVtYS5qc29uIiwiaWQiOiJmX2FkZGM0OWUyZjJmMTYyYmMiLCJsaW5lIjozMDQsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoiaW5mbyIsImNhdGVnb3J5IjoiYmFja3dhcmQtaW5jb21wYXRpYmxlIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl8zODc3M2JiMjI0MTA1MGVjIiwibGluZSI6NTQ5LCJhY3Rpb25hYmxlIjpmYWxzZX0seyJzZXZlcml0eSI6Im1lZGl1bSIsImNhdGVnb3J5IjoiYXJjaGl0ZWN0dXJhbC1jb25mbGljdCIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfNTMxMDhjZmMwMWEzZmVjMSIsImxpbmUiOjc0Mn0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoibG9naWMtZXJyb3IiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzkwMTMwZDc2ZmE0NDU1ZDkiLCJsaW5lIjo0MzV9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImVkZ2UtY2FzZSIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfMjAzZjQ3Yjc5M2JmYmIxNCIsImxpbmUiOjcxNn0seyJzZXZlcml0eSI6ImNyaXRpY2FsIiwiY2F0ZWdvcnkiOiJmYWlsLW9wZW4iLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzVmYTVhN2E2MWMzNWNmNzkiLCJsaW5lIjo1MzF9LHsic2V2ZXJpdHkiOiJoaWdoIiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfZDc4YTliZjRhYWZkMmJlOSIsImxpbmUiOjUzMX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoibG9naWMtZXJyb3IiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzc0MTg3NzBlZDg2ZDNkODYiLCJsaW5lIjo2NzZ9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InBlcm1pc3Npb24tZXhwYW5zaW9uIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl83MmM0NDJlZmMzZTk2ZTdmIiwibGluZSI6NzU5fSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiJpbnRlcm5hbC9jbGkvZGlzcGF0Y2guZ28iLCJpZCI6ImZfNGJmYzRhZTVjYTRiNzM2MSIsImxpbmUiOjE3NH0seyJzZXZlcml0eSI6Im1lZGl1bSIsImNhdGVnb3J5IjoibG9naWMtZXJyb3IiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzRmNjgyNjc5MTYxZDQwMzMiLCJsaW5lIjo3Nzd9XSwiZGlzcG9zaXRpb25zIjpbeyJpZCI6ImZfMTM4ZDg1ZjE2Y2Y5ZjAyNCIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl82ZWVkYmJkZTgxOWQ4MzdjIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmX2Q5ZDFkM2NmZmNiNWE3YTYiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfZjk0MGIxOGViNGM4ODU4YyIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl85NWE2NDg5YTlhNWIxNTU5Iiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmX2FiMWI5NGI5MzA2YWFmZWUiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfMDUxMzAxMGViMzRiZWU4MSIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl9hMWFkYjVhNDVlYjE4MWUwIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzIwMDY5Mjc5N2MyOGUyYTQiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfYWRkYzQ5ZTJmMmYxNjJiYyIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl8zODc3M2JiMjI0MTA1MGVjIiwic3RhdHVzIjoicmVjbGFzc2lmaWVkIn0seyJpZCI6ImZfNTMxMDhjZmMwMWEzZmVjMSIsInN0YXR1cyI6InJlc29sdmVkX2J5X2NoYW5nZSJ9LHsiaWQiOiJmXzkwMTMwZDc2ZmE0NDU1ZDkiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl8yMDNmNDdiNzkzYmZiYjE0Iiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn0seyJpZCI6ImZfNWZhNWE3YTYxYzM1Y2Y3OSIsInN0YXR1cyI6InJlc29sdmVkX2J5X2NoYW5nZSJ9LHsiaWQiOiJmX2Q3OGE5YmY0YWFmZDJiZTkiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl83NDE4NzcwZWQ4NmQzZDg2Iiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn0seyJpZCI6ImZfNzJjNDQyZWZjM2U5NmU3ZiIsInN0YXR1cyI6InJlc29sdmVkX2J5X2NoYW5nZSJ9LHsiaWQiOiJmXzRiZmM0YWU1Y2E0YjczNjEiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl80ZjY4MjY3OTE2MWQ0MDMzIiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn1dfQ== -->

Review

Findings

Medium

  • [logic-error] .github/workflows/reusable-dispatch.yml:771 — The authorization-metadata step relies only on ROUTE_AUTHORIZATION_DENIED and ignores the role-check, agent-check and pr-check skip outputs, so an unauthorized request for an unavailable stage (role excluded, agent disabled, existing PR) still triggers a permissions-denial comment.
    Remediation: Require that those checks did not skip the requested stage before emitting legacy authorization_denied=true; preserve confirmed CEL denials; add tests for each unavailable-stage case.
  • [test-inadequate] internal/scaffold/workflow_call_alignment_test.go:151 — The new authorization-notice job is in neither dispatchStageConcurrencyExpectations nor dispatchNonAgentJobs, so TestReusableDispatchStageConcurrency fails with "has no concurrency expectation".
    Remediation: Add it to dispatchNonAgentJobs and assert its serialization key and literal cancel-in-progress: false.
  • [misleading-label] PR title — Uses the ci prefix although the change adds user-facing behavior, touches core Go packages and extends the normative event schema; per COMMITS.md this belongs under feat/fix.
    Remediation: Retitle to feat(#7555): ...; reconsider the component/ci label.
  • [protected-path] .github/workflows/reusable-dispatch.yml — Protected path modified. The PR links dispatch: notify users when agent launch is blocked by authorization policy #7555 and explains the rationale, but human approval is always required for protected-path changes.

Low

  • [logic-error] .github/workflows/reusable-dispatch.yml:698 — PR-head resolution checks steps.route.outputs.stage rather than the kill-switch skip, so a paused authorized /fs-fix on a fork PR (or a PR lookup failure on /fs-review) still fails the run.
    Remediation: Add the kill-switch guard to this and other execution-preparation steps; test a paused /fs-fix on a fork PR.
  • [incomplete-doc] docs/guides/user/customizing-agents.md:589 — "Agent dispatch authorization" does not mention the new single deduplicated denial comment.
    Remediation: Document it under that heading.
  • [stale-doc] docs/normative/authorization/v1/README.md:409 — States unauthorized events never reach CEL evaluation; core.go now evaluates candidate triggers on unauthorized events to decide whether to notify.
    Remediation: Update the section, keeping the invariant that CEL cannot weaken authorization.
  • [fail-open] .github/workflows/reusable-dispatch.yml:684 — Pre-existing: a yq failure in the agent-enable gate exits 0 without skipped=true, so dispatch continues (only the if: changed in this PR).
  • [fail-open] .github/workflows/reusable-dispatch.yml:656 — Pre-existing: an empty roles list or yq failure skips the role membership test (identical in base; only the if: changed). Track separately.
  • [scope-creep] .github/workflows/reusable-dispatch.yml:598 — Kill-switch now skips quietly and posts a pause notice; issue dispatch: notify users when agent launch is blocked by authorization policy #7555 only names authorization denials.
  • [design-direction] .github/workflows/reusable-dispatch.yml:815 — Notices are posted from a separate runner job rather than in the binary as the issue expects for the CEL path; costs runner minutes for blocked attempts.
  • [code-organization] internal/cli/dispatch.go:144 — Event-classification and output helpers live in the CLI package; they fit normevent/harnessdispatch and internal/harnessdispatch/output.
  • [backward-incompatible] docs/normative/normalized-event/v1/normalized-event.schema.json:304 — Optional actor.numeric_id is added to a closed (additionalProperties: false) Actor object; pinned v1 validators will reject it.
    Remediation: Note in the README that v1 consumers must tolerate the field.

Earlier findings

  • f_38773bb2241050ec — reclassified (low → info): outputs are additive and an empty stage is an existing sentinel; the kill switch previously failed the job.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (5)

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl8xMzhkODVmMTZjZjlmMDI0IiwibGluZSI6NzYxLCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJtaXNsZWFkaW5nLWxhYmVsIiwiZmlsZSI6bnVsbCwiaWQiOiJmXzZlZWRiYmRlODE5ZDgzN2MiLCJhY3Rpb25hYmxlIjpmYWxzZX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoibG9naWMtZXJyb3IiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmX2Q5ZDFkM2NmZmNiNWE3YTYiLCJsaW5lIjo2ODcsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJmYWlsLW9wZW4iLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmX2Y5NDBiMThlYjRjODg1OGMiLCJsaW5lIjo2NzMsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJzY29wZS1jcmVlcCIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfOTVhNjQ4OWE5YTViMTU1OSIsImxpbmUiOjU0NCwiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImRlc2lnbi1kaXJlY3Rpb24iLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmX2FiMWI5NGI5MzA2YWFmZWUiLCJsaW5lIjo3NzQsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJjb2RlLW9yZ2FuaXphdGlvbiIsImZpbGUiOiJpbnRlcm5hbC9jbGkvZGlzcGF0Y2guZ28iLCJpZCI6ImZfMDUxMzAxMGViMzRiZWU4MSIsImxpbmUiOjE0NCwiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImluY29tcGxldGUtZG9jIiwiZmlsZSI6ImRvY3MvZ3VpZGVzL3VzZXIvY3VzdG9taXppbmctYWdlbnRzLm1kIiwiaWQiOiJmX2ExYWRiNWE0NWViMTgxZTAiLCJsaW5lIjo1ODksImFjdGlvbmFibGUiOnRydWV9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6InN0YWxlLWRvYyIsImZpbGUiOiJkb2NzL25vcm1hdGl2ZS9hdXRob3JpemF0aW9uL3YxL1JFQURNRS5tZCIsImlkIjoiZl8yMDA2OTI3OTdjMjhlMmE0IiwibGluZSI6NDA5LCJhY3Rpb25hYmxlIjp0cnVlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJiYWNrd2FyZC1pbmNvbXBhdGlibGUiLCJmaWxlIjoiZG9jcy9ub3JtYXRpdmUvbm9ybWFsaXplZC1ldmVudC92MS9ub3JtYWxpemVkLWV2ZW50LnNjaGVtYS5qc29uIiwiaWQiOiJmX2FkZGM0OWUyZjJmMTYyYmMiLCJsaW5lIjozMDQsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJiYWNrd2FyZC1pbmNvbXBhdGlibGUiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzM4NzczYmIyMjQxMDUwZWMiLCJsaW5lIjo1NDksImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJhcmNoaXRlY3R1cmFsLWNvbmZsaWN0IiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl81MzEwOGNmYzAxYTNmZWMxIiwibGluZSI6NzQyfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfOTAxMzBkNzZmYTQ0NTVkOSIsImxpbmUiOjQzNX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoiZWRnZS1jYXNlIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl8yMDNmNDdiNzkzYmZiYjE0IiwibGluZSI6NzE2fSx7InNldmVyaXR5IjoiY3JpdGljYWwiLCJjYXRlZ29yeSI6ImZhaWwtb3BlbiIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfNWZhNWE3YTYxYzM1Y2Y3OSIsImxpbmUiOjUzMX0seyJzZXZlcml0eSI6ImhpZ2giLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl9kNzhhOWJmNGFhZmQyYmU5IiwibGluZSI6NTMxfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfNzQxODc3MGVkODZkM2Q4NiIsImxpbmUiOjY3Nn0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoicGVybWlzc2lvbi1leHBhbnNpb24iLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzcyYzQ0MmVmYzNlOTZlN2YiLCJsaW5lIjo3NTl9LHsic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6ImludGVybmFsL2NsaS9kaXNwYXRjaC5nbyIsImlkIjoiZl80YmZjNGFlNWNhNGI3MzYxIiwibGluZSI6MTc0fSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfNGY2ODI2NzkxNjFkNDAzMyIsImxpbmUiOjc3N31dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl9kOWQxZDNjZmZjYjVhN2E2Iiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzZlZWRiYmRlODE5ZDgzN2MiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfOTVhNjQ4OWE5YTViMTU1OSIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl9hYjFiOTRiOTMwNmFhZmVlIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmX2ExYWRiNWE0NWViMTgxZTAiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfMjAwNjkyNzk3YzI4ZTJhNCIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl9hZGRjNDllMmYyZjE2MmJjIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzM4NzczYmIyMjQxMDUwZWMiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfZjk0MGIxOGViNGM4ODU4YyIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl81MzEwOGNmYzAxYTNmZWMxIiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn0seyJpZCI6ImZfOTAxMzBkNzZmYTQ0NTVkOSIsInN0YXR1cyI6InJlc29sdmVkX2J5X2NoYW5nZSJ9LHsiaWQiOiJmXzIwM2Y0N2I3OTNiZmJiMTQiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl81ZmE1YTdhNjFjMzVjZjc5Iiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn0seyJpZCI6ImZfZDc4YTliZjRhYWZkMmJlOSIsInN0YXR1cyI6InJlc29sdmVkX2J5X2NoYW5nZSJ9LHsiaWQiOiJmXzc0MTg3NzBlZDg2ZDNkODYiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl83MmM0NDJlZmMzZTk2ZTdmIiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn0seyJpZCI6ImZfNGJmYzRhZTVjYTRiNzM2MSIsInN0YXR1cyI6InJlc29sdmVkX2J5X2NoYW5nZSJ9LHsiaWQiOiJmXzRmNjgyNjc5MTYxZDQwMzMiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifV19 -->

Review

Findings

Medium

  • [logic-error] .github/workflows/reusable-dispatch.yml:761 — The legacy kill-switch notification branch in the Emit authorization denial metadata step is unreachable. The guard before it (lines 747-759) exits 0 whenever KILL_SWITCH_SKIPPED == "true", and whenever STAGE is non-empty. The block that writes dispatch_blocked_reason=kill_switch can therefore never run, so legacy-only installs never emit the pause notice the PR advertises.
    Remediation: Separate the common human-command validation from the denial-specific guards. Drop KILL_SWITCH_SKIPPED == true and the -n STAGE check from the early-exit guard, or test the kill-switch case before it. Add an executable workflow-step test for a paused request without CEL triggers.
  • [protected-path] .github/workflows/reusable-dispatch.yml — This PR modifies a protected path: it adds the authorization-notice job with issues: write, new route outputs, and changed kill-switch behavior. The PR links dispatch: notify users when agent launch is blocked by authorization policy #7555 and explains the rationale, but human approval is always required for protected-path changes, regardless of context.
  • [misleading-label] PR-level (f_6eedbbde819d837c) — The ci(#7555): title prefix and component/ci label understate a change that touches Go production code (internal/harnessdispatch/core.go, internal/cli/dispatch.go, internal/normevent) and adds a user-visible comment. COMMITS.md excludes ci from release notes.
    Remediation: Retitle with feat(#7555): (or fix(#7555):).

Low

  • [logic-error] .github/workflows/reusable-dispatch.yml:687 (f_d9d1d3cffcb5a7a6) — The Resolve PR head for issue_comment events step does not check steps.kill-switch.outputs.skipped. During a kill-switch pause, an authorized /fs-fix on a fork PR, or a PR API failure, can still fail the routing job, which defeats the new quiet-skip behavior.
    Remediation: Add steps.kill-switch.outputs.skipped != 'true' to the step's if:.
  • [fail-open] .github/workflows/reusable-dispatch.yml:673 (f_f940b18eb4c8858c) — The Check agent is enabled step exits 0 without skipped=true when its yq query fails. This logic is unchanged by the PR (only the if: and STAGE source changed), so the severity stays at the prior low rating.
  • [scope-creep] .github/workflows/reusable-dispatch.yml:544 (f_95a6489a9a5b1559) — The kill-switch quiet-skip, the fullsend:dispatch-paused notice, and the kill_switch blocked reason in core.go and dispatch.go go beyond dispatch: notify users when agent launch is blocked by authorization policy #7555, which authorizes authorization-denial notices only.
    Remediation: Split them into a separate issue and PR.
  • [design-direction] .github/workflows/reusable-dispatch.yml:774 (f_ab1b94b9306aafee) — The authorization step infers a denial from an empty STAGE plus a dozen skipped/outcome conditions, instead of route recording the denial where is_authorized fails. This is fragile, and it contributed to the unreachable branch above.
  • [code-organization] internal/cli/dispatch.go:144 — writeAuthorizationOutput is never called from production code. Only dispatch_test.go uses it.
    Remediation: Move it into the test file, or call writeDispatchOutput(event, denied, "") in the tests.
  • [incomplete-doc] docs/guides/user/customizing-agents.md:589 (f_a1adb5a45eb181e0) — The 'Agent dispatch authorization' section does not mention the first-denial comment or per-actor deduplication.
  • [stale-doc] docs/normative/authorization/v1/README.md:409 (f_200692797c28e2a4) — It says unauthorized events never reach CEL evaluation. The dispatch core now evaluates candidate triggers for an unauthorized event, to build the denial notice, before returning AuthorizationDenied.
  • [backward-incompatible] docs/normative/normalized-event/v1/normalized-event.schema.json:304 (f_addc49e2f2f162bc) — The optional actor.numeric_id is added to a v1 schema whose actor has additionalProperties: false. Consumers pinning an older vendored copy will reject events that carry it.
  • [backward-incompatible] .github/workflows/reusable-dispatch.yml:549 (f_38773bb2241050ec) — An active kill switch used to fail the routing job. It now succeeds with an empty stage, so monitors that relied on the failure will see a green run.
Previous run (6)

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl9kOWQxZDNjZmZjYjVhN2E2IiwibGluZSI6NjA0fSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJtaXNsZWFkaW5nLWxhYmVsIiwiZmlsZSI6bnVsbCwiaWQiOiJmXzZlZWRiYmRlODE5ZDgzN2MifSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJzY29wZS1jcmVlcCIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfOTVhNjQ4OWE5YTViMTU1OSIsImxpbmUiOjU0NH0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoiZGVzaWduLWRpcmVjdGlvbiIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfYWIxYjk0YjkzMDZhYWZlZSIsImxpbmUiOjc3NH0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoiaW5jb21wbGV0ZS1kb2MiLCJmaWxlIjoiZG9jcy9ndWlkZXMvdXNlci9jdXN0b21pemluZy1hZ2VudHMubWQiLCJpZCI6ImZfYTFhZGI1YTQ1ZWIxODFlMCIsImxpbmUiOjU4OX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5Ijoic3RhbGUtZG9jIiwiZmlsZSI6ImRvY3Mvbm9ybWF0aXZlL2F1dGhvcml6YXRpb24vdjEvUkVBRE1FLm1kIiwiaWQiOiJmXzIwMDY5Mjc5N2MyOGUyYTQiLCJsaW5lIjozNjd9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImJhY2t3YXJkLWluY29tcGF0aWJsZSIsImZpbGUiOiJkb2NzL25vcm1hdGl2ZS9ub3JtYWxpemVkLWV2ZW50L3YxL25vcm1hbGl6ZWQtZXZlbnQuc2NoZW1hLmpzb24iLCJpZCI6ImZfYWRkYzQ5ZTJmMmYxNjJiYyIsImxpbmUiOjMwNH0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoiYmFja3dhcmQtaW5jb21wYXRpYmxlIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl8zODc3M2JiMjI0MTA1MGVjIiwibGluZSI6NTQ5fSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJmYWlsLW9wZW4iLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmX2Y5NDBiMThlYjRjODg1OGMiLCJsaW5lIjo2MTB9LHsic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImFyY2hpdGVjdHVyYWwtY29uZmxpY3QiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzUzMTA4Y2ZjMDFhM2ZlYzEiLCJsaW5lIjo3NDJ9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl85MDEzMGQ3NmZhNDQ1NWQ5IiwibGluZSI6NDM1fSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJlZGdlLWNhc2UiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzIwM2Y0N2I3OTNiZmJiMTQiLCJsaW5lIjo3MTZ9LHsic2V2ZXJpdHkiOiJjcml0aWNhbCIsImNhdGVnb3J5IjoiZmFpbC1vcGVuIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl81ZmE1YTdhNjFjMzVjZjc5IiwibGluZSI6NTMxfSx7InNldmVyaXR5IjoiaGlnaCIsImNhdGVnb3J5IjoibG9naWMtZXJyb3IiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmX2Q3OGE5YmY0YWFmZDJiZTkiLCJsaW5lIjo1MzF9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl83NDE4NzcwZWQ4NmQzZDg2IiwibGluZSI6Njc2fSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJwZXJtaXNzaW9uLWV4cGFuc2lvbiIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfNzJjNDQyZWZjM2U5NmU3ZiIsImxpbmUiOjc1OX0seyJzZXZlcml0eSI6Im1lZGl1bSIsImNhdGVnb3J5IjoibG9naWMtZXJyb3IiLCJmaWxlIjoiaW50ZXJuYWwvY2xpL2Rpc3BhdGNoLmdvIiwiaWQiOiJmXzRiZmM0YWU1Y2E0YjczNjEiLCJsaW5lIjoxNzR9LHsic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl80ZjY4MjY3OTE2MWQ0MDMzIiwibGluZSI6Nzc3fV0sImRpc3Bvc2l0aW9ucyI6W3siaWQiOiJmXzRmNjgyNjc5MTYxZDQwMzMiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl82ZWVkYmJkZTgxOWQ4MzdjIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmX2Q5ZDFkM2NmZmNiNWE3YTYiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfOTVhNjQ4OWE5YTViMTU1OSIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl9hYjFiOTRiOTMwNmFhZmVlIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmX2FkZGM0OWUyZjJmMTYyYmMiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfMzg3NzNiYjIyNDEwNTBlYyIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl9hMWFkYjVhNDVlYjE4MWUwIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmXzUzMTA4Y2ZjMDFhM2ZlYzEiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl85MDEzMGQ3NmZhNDQ1NWQ5Iiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn0seyJpZCI6ImZfMjAzZjQ3Yjc5M2JmYmIxNCIsInN0YXR1cyI6InJlc29sdmVkX2J5X2NoYW5nZSJ9LHsiaWQiOiJmXzVmYTVhN2E2MWMzNWNmNzkiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl9kNzhhOWJmNGFhZmQyYmU5Iiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn0seyJpZCI6ImZfNzQxODc3MGVkODZkM2Q4NiIsInN0YXR1cyI6InJlc29sdmVkX2J5X2NoYW5nZSJ9LHsiaWQiOiJmXzcyYzQ0MmVmYzNlOTZlN2YiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl80YmZjNGFlNWNhNGI3MzYxIiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn1dfQ== -->

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml — The PR modifies this protected governance/infrastructure file (new issues: write job, kill-switch semantics, route outputs). It links dispatch: notify users when agent launch is blocked by authorization policy #7555 and explains the rationale, but human approval is always required for protected-path changes.
  • [misleading-label] PR title — ci(#7555): keeps a user-visible behavior (a new comment on unauthorized launch) out of the release notes per COMMITS.md.
    Remediation: Retitle with a feat(#7555): prefix.

Low

  • [logic-error] .github/workflows/reusable-dispatch.yml:604 — Resolve PR head for issue_comment events still runs after an active kill switch. Role-check and agent-check are skipped by condition and emit empty skipped outputs, so the step's if: passes. An authorized /fs-fix on a fork PR, or /fs-review with a failed PR lookup, still fails the routing job instead of the intended clean skip.
    Remediation: Add steps.kill-switch.outputs.skipped != 'true' to the step's if: and assert it in the workflow tests.
  • [stale-doc] docs/normative/authorization/v1/README.md:367 — The spec says an unauthorized event never reaches CEL evaluation. internal/harnessdispatch/core.go now calls MatchHarnesses for unauthorized events so AuthorizationDenied can be set (no harness is executed).
    Remediation: Update the passage to say triggers are evaluated for unauthorized events only to detect a matching launch attempt for denial notification.
  • [incomplete-doc] docs/guides/user/customizing-agents.md:589 — The Agent dispatch authorization section does not mention the single deduplicated denial comment, which is documented only in cel-triggers-reference.md; legacy slash commands also post it.
    Remediation: Add a short note under that section.
  • [scope-creep] .github/workflows/reusable-dispatch.yml:544 — The kill switch changes from failing the routing job to a clean skip. The notice job does not need this change, and dispatch: notify users when agent launch is blocked by authorization policy #7555 covers only authorization-denial notices.
  • [design-direction] .github/workflows/reusable-dispatch.yml:774 — dispatch: notify users when agent launch is blocked by authorization policy #7555 says the binary should post the comment for CEL agents. Here the binary only emits metadata and the authorization-notice job posts via fullsend issues post-comment. This centralizes dedup but departs from the issue and ties the notice to GitHub Actions.
  • [backward-incompatible] docs/normative/normalized-event/v1/normalized-event.schema.json:304 — Optional actor.numeric_id is added to v1 where actor has additionalProperties: false. Consumers with a pinned strict older copy will reject events that carry it.
    Remediation: Confirm no such consumer exists, or note the additive change in the spec changelog.
  • [backward-incompatible] .github/workflows/reusable-dispatch.yml:549 — The route job exposes extra outputs. workflow_call inputs, secrets and outputs are unchanged, so only callers reading route outputs could be affected.
  • [fail-open] .github/workflows/reusable-dispatch.yml:610 — Inherited, not changed by this PR: an explicit roles: [] (or a failed yq query) gives an empty ROLES, which skips the role membership check and leaves every legacy stage enabled. This conflicts with the explicit-empty semantics in internal/config.
    Remediation: In a follow-up, separate an omitted roles key from an empty list and treat query failures as skip or error.

Earlier findings

  • f_4f682679161d4033 — resolved_by_change: the added needs.route.outputs.stage == '' disjunct in the authorization-notice if: (plus the matching test assertion) lets a confirmed CEL denial through when an applicable legacy stage is unavailable.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (7)

<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl80ZjY4MjY3OTE2MWQ0MDMzIiwibGluZSI6Nzc3fSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJtaXNsZWFkaW5nLWxhYmVsIiwiZmlsZSI6bnVsbCwiaWQiOiJmXzZlZWRiYmRlODE5ZDgzN2MifSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfZDlkMWQzY2ZmY2I1YTdhNiIsImxpbmUiOjY1MX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5Ijoic2NvcGUtY3JlZXAiLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzk1YTY0ODlhOWE1YjE1NTkiLCJsaW5lIjo1NDR9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImRlc2lnbi1kaXJlY3Rpb24iLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmX2FiMWI5NGI5MzA2YWFmZWUiLCJsaW5lIjo3NzR9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImJhY2t3YXJkLWluY29tcGF0aWJsZSIsImZpbGUiOiJkb2NzL25vcm1hdGl2ZS9ub3JtYWxpemVkLWV2ZW50L3YxL25vcm1hbGl6ZWQtZXZlbnQuc2NoZW1hLmpzb24iLCJpZCI6ImZfYWRkYzQ5ZTJmMmYxNjJiYyIsImxpbmUiOjMwNH0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoiYmFja3dhcmQtaW5jb21wYXRpYmxlIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl8zODc3M2JiMjI0MTA1MGVjIiwibGluZSI6NTQ5fSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJpbmNvbXBsZXRlLWRvYyIsImZpbGUiOiJkb2NzL2d1aWRlcy91c2VyL2N1c3RvbWl6aW5nLWFnZW50cy5tZCIsImlkIjoiZl9hMWFkYjVhNDVlYjE4MWUwIiwibGluZSI6NTg5fSx7InNldmVyaXR5IjoibWVkaXVtIiwiY2F0ZWdvcnkiOiJhcmNoaXRlY3R1cmFsLWNvbmZsaWN0IiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl81MzEwOGNmYzAxYTNmZWMxIiwibGluZSI6NzQyfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfOTAxMzBkNzZmYTQ0NTVkOSIsImxpbmUiOjQzNX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoiZWRnZS1jYXNlIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl8yMDNmNDdiNzkzYmZiYjE0IiwibGluZSI6NzE2fSx7InNldmVyaXR5IjoiY3JpdGljYWwiLCJjYXRlZ29yeSI6ImZhaWwtb3BlbiIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfNWZhNWE3YTYxYzM1Y2Y3OSIsImxpbmUiOjUzMX0seyJzZXZlcml0eSI6ImhpZ2giLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6Ii5naXRodWIvd29ya2Zsb3dzL3JldXNhYmxlLWRpc3BhdGNoLnltbCIsImlkIjoiZl9kNzhhOWJmNGFhZmQyYmU5IiwibGluZSI6NTMxfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJsb2dpYy1lcnJvciIsImZpbGUiOiIuZ2l0aHViL3dvcmtmbG93cy9yZXVzYWJsZS1kaXNwYXRjaC55bWwiLCJpZCI6ImZfNzQxODc3MGVkODZkM2Q4NiIsImxpbmUiOjY3Nn0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoicGVybWlzc2lvbi1leHBhbnNpb24iLCJmaWxlIjoiLmdpdGh1Yi93b3JrZmxvd3MvcmV1c2FibGUtZGlzcGF0Y2gueW1sIiwiaWQiOiJmXzcyYzQ0MmVmYzNlOTZlN2YiLCJsaW5lIjo3NTl9LHsic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImxvZ2ljLWVycm9yIiwiZmlsZSI6ImludGVybmFsL2NsaS9kaXNwYXRjaC5nbyIsImlkIjoiZl80YmZjNGFlNWNhNGI3MzYxIiwibGluZSI6MTc0fV0sImRpc3Bvc2l0aW9ucyI6W3siaWQiOiJmXzRiZmM0YWU1Y2E0YjczNjEiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl82ZWVkYmJkZTgxOWQ4MzdjIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmX2FiMWI5NGI5MzA2YWFmZWUiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfYWRkYzQ5ZTJmMmYxNjJiYyIsInN0YXR1cyI6Im9wZW4ifSx7ImlkIjoiZl8zODc3M2JiMjI0MTA1MGVjIiwic3RhdHVzIjoib3BlbiJ9LHsiaWQiOiJmX2ExYWRiNWE0NWViMTgxZTAiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfNTMxMDhjZmMwMWEzZmVjMSIsInN0YXR1cyI6InJlc29sdmVkX2J5X2NoYW5nZSJ9LHsiaWQiOiJmXzkwMTMwZDc2ZmE0NDU1ZDkiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl8yMDNmNDdiNzkzYmZiYjE0Iiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn0seyJpZCI6ImZfNWZhNWE3YTYxYzM1Y2Y3OSIsInN0YXR1cyI6InJlc29sdmVkX2J5X2NoYW5nZSJ9LHsiaWQiOiJmX2Q3OGE5YmY0YWFmZDJiZTkiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl83NDE4NzcwZWQ4NmQzZDg2Iiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn0seyJpZCI6ImZfNzJjNDQyZWZjM2U5NmU3ZiIsInN0YXR1cyI6InJlc29sdmVkX2J5X2NoYW5nZSJ9XX0= -->

Review

Findings

Medium

  • [logic-error] .github/workflows/reusable-dispatch.yml:777 — A recognized legacy command suppresses independently confirmed CEL denials even when the legacy stage is unavailable. For example, disable the legacy code agent and register an enabled custom harness triggered by /fs-code. DispatchResult reports an authorization denial for an unauthorized user's matching custom harness, but route emits no denial because the legacy agent is skipped, and legacy_command_applicable=true stops authorization-notice from consuming the CEL result. The custom harness can run for authorized users while denied users get no explanation.
    Remediation: Let confirmed harness-dispatch denials trigger the shared notification job regardless of legacy command applicability. Legacy availability checks should suppress only legacy-generated denial metadata; keep the single serialized comment producer for deduplication.
  • [misleading-label] N/A (f_6eedbbde819d837c) — The PR title ci(#7555): notify users when agent launch is unauthorized misclassifies the change under COMMITS.md. Posting a denial comment to users is new user-facing behavior and touches Go production packages (internal/cli, internal/harnessdispatch, internal/normevent) and the normative event schema, not just CI. ci is excluded from release notes; this fits feat. The kill-switch change (failing routing job -> successful skip) alters observable caller behavior, so consider the ! marker and a BREAKING CHANGE trailer (COMMITS.md: mark when unsure).
    Remediation: Retitle to feat(#7555): ... (with ! and a BREAKING CHANGE trailer if the kill-switch contract change is judged breaking).
  • [protected-path] .github/workflows/reusable-dispatch.yml — This PR modifies protected path .github/workflows/reusable-dispatch.yml (governance/infrastructure). The PR links dispatch: notify users when agent launch is blocked by authorization policy #7555 and explains the rationale, but human approval is always required for protected-path changes regardless of context.

Low

  • [logic-error] .github/workflows/reusable-dispatch.yml:651 — The new successful kill-switch skip does not stop the later PR-head step. An authorized /fs-fix or /fs-review still leaves steps.route.outputs.stage populated, while skipped role/agent checks have empty outputs that satisfy this step's condition. A fork PR's /fs-fix or a failed PR lookup can therefore still fail Route after the kill switch has recorded a successful skip. The build-payload step (stage != '') also ignores the kill switch.
    Remediation: Include steps.kill-switch.outputs.skipped != 'true' in the PR-head and payload step conditions, and test that an active kill switch skips downstream stage preparation without failing Route.
  • [scope-creep] .github/workflows/reusable-dispatch.yml:544 — Changing the kill switch from failing the routing job (exit 1) to a clean skip (skipped=true, exit 0) is not requested by issue dispatch: notify users when agent launch is blocked by authorization policy #7555. It is disclosed in the PR body and loosely tied to the feature, but the authorization step already requires KILL_SWITCH_OUTCOME == success, so suppressing false denials did not need the contract change.
    Remediation: Either keep the prior exit-1 contract or justify the contract change explicitly and treat it as a separately reviewed behavior change.
  • [design-direction] .github/workflows/reusable-dispatch.yml:774 (f_ab1b94b9306aafee) — Issue dispatch: notify users when agent launch is blocked by authorization policy #7555 says CEL-based agents' denials should be posted by the binary. The implementation adds a separate authorization-notice job using fullsend issues post-comment for both legacy and CEL paths; the binary only writes authorization_* outputs. This is documented in docs/contributing/ci-workflows.md as intentional serialization, but it diverges from the issue's wording.
  • [backward-incompatible] docs/normative/normalized-event/v1/normalized-event.schema.json:304 (f_addc49e2f2f162bc) — actor.numeric_id is added to the v1 schema while actor keeps additionalProperties: false. Strict validators pinned to an older v1 copy will reject payloads containing the field. The README now documents this as an additive v1.x change, but it remains a contract change for external consumers of the closed schema.
    Remediation: No blocker. Announce in release notes and confirm no sibling-repo schema copies need the field.
  • [backward-incompatible] .github/workflows/reusable-dispatch.yml:549 (f_38773bb2241050ec) — An active kill switch now makes the routing job succeed with an empty stage output instead of failing. Caller repos (shims, scaffold) that treated a failed routing job as the stop signal, or gate follow-on jobs on result == 'success' without checking stage != '', will now see success. config-reference.md now tells callers to use the stage output; caller gating was not verified.
    Remediation: Confirm scaffold and shim callers gate on the stage output; mention in release notes.
  • [incomplete-doc] docs/guides/user/customizing-agents.md:589 (f_a1adb5a45eb181e0) — The 'Agent dispatch authorization' section describes permission thresholds for slash commands and webhooks but does not document the denial notification comment (first unauthorized attempt, deduplicated per actor ID) introduced by this PR.
    Remediation: Update that section to describe the denial comment, its deduplication, and that an active kill switch skips dispatch without a denial notice.

Earlier findings

  • f_4bfc4ae5ca4b7361 — resolved_by_change: isExplicitHumanSlashCommand in internal/cli/dispatch.go now accepts any non-empty slash command, and dispatch_test.go uses /analyze.

Labels: PR changes the reusable CI dispatch workflow and Go dispatch authorization code


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

Notify only for confirmed, applicable launch denials and centralize comment creation in the workflow.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 12:20 PM UTC · Ended 12:22 PM UTC

Commit: 6d7ac36 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 12:24 PM UTC · Ended 12:37 PM UTC

Commit: d502281 · View workflow run →

@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.95041% with 17 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/harnessdispatch/core.go 77.55% 7 Missing and 4 partials ⚠️
internal/cli/dispatch.go 84.21% 3 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:24 PM UTC · Completed 12:37 PM UTC

Commit: d502281 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $3.65

Group related GITHUB_OUTPUT writes in the authorization route.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

Resolve reusable-dispatch workflow conflicts while preserving authorization notices and current-stage gating.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
Evaluate registered harness triggers while the kill switch is active so unmatched slash commands do not receive agent-pause notices. Add coverage for matched and unmatched commands, normalize dispatch test names, and verify the unauthorized behaviour path posts its issue comment.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:43 AM UTC · Completed 12:21 PM UTC

Commit: ee95fda · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $3.25

@fullsend-ai-review fullsend-ai-review Bot added risk/elevated PR risk: elevated and removed risk/moderate PR risk: moderate labels Oct 22, 2026
fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

The existing-PR gate was removed from main in fullsend-ai#8133 but reintroduced by the workflow changes for fullsend-ai#7555. Drop the gate and its authorization metadata/test plumbing so code dispatch is not incorrectly suppressed by unrelated open PRs.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:30 PM UTC · Completed 8:43 PM UTC

Commit: 4a271d7 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $3.10

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

…atches

Require the pre-CEL authorization gate and a matching harness trigger before emitting denial metadata. Keep unauthorized harnesses out of execution refs while using a non-dispatching probe to distinguish denied launches from unrelated events.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:47 AM UTC · Completed 1:04 AM UTC

Commit: 6060bd0 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $4.02

@fullsend-ai-review fullsend-ai-review 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.

Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:

  • .github/workflows/reusable-dispatch.yml (file-level): Line 649 ·
    [medium] logic-error

The "Resolve PR head for issue_comment events" step still runs after the kill switch skips dispatch. Its if: checks steps.route.outputs.stage != '' plus the role-check/agent-check skipped outputs, but not steps.kill-switch.outputs.skipped. Those two checks are themselves gated on the kill switch, so their outputs are empty when it fires and do not stop this step. The route step's own stage output also stays non-empty; only the job-level stage output (line 122) applies the kill-switch guard. As a result, an authorized /fs-fix on a fork PR fails the route job at the fork guard (exit 1) while dispatch is paused, and a PR API failure for fix/review stages fails it the same way. This contradicts the new successful-skip behavior.

Suggested fix: Add steps.kill-switch.outputs.skipped != 'true' to this step's if: and to any other post-routing execution-preparation steps that key on steps.route.outputs.stage. Add a test showing a paused /fs-fix request on a fork PR skips PR resolution and leaves routing successful.

@fullsend-ai-review fullsend-ai-review 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.

See the review comment for full details.

if err != nil {
return Result{}, err
}
authorizationDenied = len(deniedMatches) > 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] edge-case

Per-harness denial metadata ignores AuthorizationAvailable(). When the live collaborator-permission lookup fails (role stays none, authorizationUnavailable is set) but an OWNERS file upgrades the effective role to triage, IsEventAuthorized(&authCheck) passes and the !eventAuthorized && !AuthorizationAvailable() short-circuit is skipped. A matching harness with a higher role threshold (for example a coder harness behind a custom slash command) then sets AuthorizationDenied=true, so the actor gets a "not authorized" comment even though their real permissions were never resolved and could include write access. Execution stays fail-closed; the defect is a false denial notice. The existing test covers an unavailable lookup only without an OWNERS grant.

Suggested fix: Suppress the per-harness authorizationDenied result when opts.Event.AuthorizationAvailable() is false, while keeping execution that OWNERS grants independently allow. Add a test combining MarkAuthorizationUnavailable(), an OWNERS reviewer entry, and a matching write-threshold slash-command harness.

@@ -490,13 +541,17 @@ jobs:
KILL_SWITCH="${KILL_SWITCH#\"}"
KILL_SWITCH="${KILL_SWITCH%\"}"
if [[ "$KILL_SWITCH" == "true" || "$KILL_SWITCH" == "yes" ]]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] scope-creep

Issue #7555 covers notifying users when authorization policy blocks a launch. The PR also changes kill-switch behavior from failing the routing job (exit 1) to a quiet skip, adds a separate paused-dispatch notice (fullsend:dispatch-paused:<actor id>), makes internal/harnessdispatch/core.go evaluate triggers while the kill switch is active, and amends docs/reference/config-reference.md. The PR body discloses this, but the linked issue does not authorize it.

Suggested fix: Descope the kill-switch pause behavior, or file and link an issue that authorizes it.

"maxLength": 255,
"description": "Forge login or service account identifier."
},
"numeric_id": {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] breaking-schema

The new optional actor.numeric_id is additive (not in required), but the actor object sets additionalProperties: false, so consumers validating against a pinned copy of the v1 schema will reject events that carry the field. The README documents it as an additive v1.x change and warns pinned-copy consumers; the schema $id and version are unchanged.

Suggested fix: Keep the README note; consider a schema changelog entry.

@@ -490,13 +541,17 @@ jobs:
KILL_SWITCH="${KILL_SWITCH#\"}"
KILL_SWITCH="${KILL_SWITCH%\"}"
if [[ "$KILL_SWITCH" == "true" || "$KILL_SWITCH" == "yes" ]]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] backward-incompatible

An active kill switch now makes the route job succeed with an empty stage output; before this change the job failed. Callers that treated route-job success as "dispatch proceeded" must now gate on stage != ''. The change is documented in ci-workflows.md and config-reference.md.

Suggested fix: Call out the change in release notes and confirm the shim and scaffold workflows gate on the stage output.

This branch was successfully deployed

2 active deployments
site-preview — 6060bd04 Deployed Oct 23, 2026 by github-actions[bot]
dev — 6060bd04 Deployed Oct 23, 2026 by ifireball via playback #17722
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/ci CI pipelines and checks component/dispatch Workflow dispatch and triggers risk/elevated PR risk: elevated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dispatch: notify users when agent launch is blocked by authorization policy

2 participants