Skip to content

docs(authorization)!: define bot identity resolution contract - #7765

Merged
ifireball merged 25 commits into
fullsend-ai:mainfrom
ifireball:codex/bot-identity-authorization
Oct 7, 2026
Merged

ifireball merged 25 commits into
fullsend-ai:mainfrom
ifireball:codex/bot-identity-authorization

Conversation

@ifireball

Copy link
Copy Markdown
Member

Summary

  • Add ADR 0107 defining provider-backed bot identity resolution before dispatch authorization.
  • Separate actor.kind, actor.bot_role, and human permission actor.role; bot actors carry role: null.
  • Align the normalized-event schema, authorization contract, adapters, fixtures, and CEL documentation.
  • Record the historical authorization drift from ADR 0054 and link the motivating issue.

Historical motivation

The design is motivated by #7764, which records the changes since ADR 0054: split observation/mutation thresholds, label and review exceptions for App bots, the OWNERS provider, removal of the weaker needs-info gate, the normative authorization contract, and the eventless path in ADR 0098.

Verification

  • make lint

Closes #7764.

Record ADR 0107 as the basis for the next authorization decision and align
the normalized-event schema, authorization contract, adapters, fixtures, and
CEL documentation with separate human-permission and bot-role fields.

Refs fullsend-ai#7764.

BREAKING CHANGE: Bot actors now carry a null permission role and a separate
provider-resolved bot role; normalized-event producers and consumers must
adopt the updated contract.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@ifireball
ifireball requested a review from a team September 27, 2026 12:56
@ifireball ifireball self-assigned this Sep 27, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Define provider-backed bot identity for dispatch authorization

📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Adopt ADR 0107’s trusted bot-role lookup before dispatch authorization, replacing generic bot
 exceptions after migration.
• Separate human permission roles from bot roles in the normalized-event schema and examples.
• Align authorization, adapter, architecture, and CEL documentation with the new contract.
Diagram

graph TD
  Adapter["Forge adapter"] --> Provider["Identity provider"] --> Actor["Normalized actor"] --> Gate{"Dispatch gate"} --> CEL["CEL routing"] --> Agent["Agent dispatch"]
  Adapter --> Permissions["Human permissions"] --> Actor
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Introduce NormalizedEvent v2
  • ➕ Preserves existing v1 producers and consumers during migration.
  • ➕ Follows the documented rule that breaking schema changes require v2.
  • ➖ Requires temporary support for two event contracts.
  • ➖ Adds migration work before the new fields can become mandatory.

Recommendation: Keep ADR 0107’s provider-backed identity strategy, but consider publishing the required bot_role field and changed role semantics as NormalizedEvent v2. The current v1 versioning policy explicitly reserves breaking changes for v2, and runtime adapters and authorization still implement the compatibility behavior.

Files changed (21) +238 / -35

Documentation (20) +204 / -30
0054-require-authorization-on-all-agent-dispatch-paths.mdLink the successor bot-identity decision +9/-0

Link the successor bot-identity decision

• Points readers to ADR 0107 and distinguishes the target fail-closed contract from existing bot compatibility behavior.

docs/ADRs/0054-require-authorization-on-all-agent-dispatch-paths.md

0107-bot-identity-resolution-for-dispatch-authorization.mdAdopt provider-backed bot identity resolution +127/-0

Adopt provider-backed bot identity resolution

• Defines exact registered-identity lookup, distinct human and bot roles, fail-closed authorization, and CEL’s narrower routing role. Records the migration boundary and motivating issue.

docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md

architecture.mdDescribe the target bot authorization flow +9/-3

Describe the target bot authorization flow

• Adds provider-backed bot-role resolution to the architecture overview while identifying it as a migration.

docs/architecture.md

cel-triggers-reference.mdDocument separate CEL actor role fields +2/-1

Document separate CEL actor role fields

• Clarifies that actor.role is null for bots and exposes the nullable, provider-resolved actor.bot_role field.

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

README.mdSpecify identity-based bot authorization +30/-14

Specify identity-based bot authorization

• Documents the bot field invariants, fail-closed gate, and post-migration handling of bot reviews and service identities.

docs/normative/authorization/v1/README.md

README.mdDefine normalized human and bot role meanings +10/-9

Define normalized human and bot role meanings

• Replaces bot permission-role guidance with provider-resolved bot roles, including for schedule and manual sources.

docs/normative/normalized-event/v1/README.md

discussion-fs-vouch-comment.jsonAdd null bot role to discussion example +1/-0

Add null bot role to discussion example

• Makes the human actor’s bot_role explicitly null.

docs/normative/normalized-event/v1/examples/discussion-fs-vouch-comment.json

fs-fix-comment.jsonAdd null bot role to fix-comment example +1/-0

Add null bot role to fix-comment example

• Makes the human actor’s bot_role explicitly null.

docs/normative/normalized-event/v1/examples/fs-fix-comment.json

gitlab-mr-merged.jsonAdd null bot role to merged-MR example +1/-0

Add null bot role to merged-MR example

• Makes the GitLab human actor’s bot_role explicitly null.

docs/normative/normalized-event/v1/examples/gitlab-mr-merged.json

gitlab-mr-opened.jsonAdd null bot role to opened-MR example +1/-0

Add null bot role to opened-MR example

• Makes the GitLab human actor’s bot_role explicitly null.

docs/normative/normalized-event/v1/examples/gitlab-mr-opened.json

invalid-path-traversal.jsonPreserve invalid fixture under the new actor shape +1/-0

Preserve invalid fixture under the new actor shape

• Adds a null bot_role to the human actor without changing the fixture’s path-traversal case.

docs/normative/normalized-event/v1/examples/invalid-path-traversal.json

issue-opened.jsonAdd null bot role to issue example +1/-0

Add null bot role to issue example

• Makes the human reporter’s bot_role explicitly null.

docs/normative/normalized-event/v1/examples/issue-opened.json

jira-fs-triage-comment.jsonAdd null bot role to Jira comment example +1/-0

Add null bot role to Jira comment example

• Makes the Jira human actor’s bot_role explicitly null.

docs/normative/normalized-event/v1/examples/jira-fs-triage-comment.json

pr-closed.jsonAdd null bot role to closed-PR example +1/-0

Add null bot role to closed-PR example

• Makes the human actor’s bot_role explicitly null.

docs/normative/normalized-event/v1/examples/pr-closed.json

pr-opened-fork.jsonAdd null bot role to fork-PR example +1/-0

Add null bot role to fork-PR example

• Makes the external human actor’s bot_role explicitly null.

docs/normative/normalized-event/v1/examples/pr-opened-fork.json

pr-opened.jsonAdd null bot role to opened-PR example +1/-0

Add null bot role to opened-PR example

• Makes the human actor’s bot_role explicitly null.

docs/normative/normalized-event/v1/examples/pr-opened.json

pr-synchronized.jsonAdd null bot role to synchronized-PR example +1/-0

Add null bot role to synchronized-PR example

• Makes the human actor’s bot_role explicitly null.

docs/normative/normalized-event/v1/examples/pr-synchronized.json

ready-to-code-labeled.jsonSeparate triage bot identity from permission +2/-1

Separate triage bot identity from permission

• Changes the bot’s permission role to null and assigns the canonical triage bot_role.

docs/normative/normalized-event/v1/examples/ready-to-code-labeled.json

review-changes-requested.jsonSeparate review bot identity from permission +2/-1

Separate review bot identity from permission

• Changes the bot’s permission role to null and assigns the canonical review bot_role.

docs/normative/normalized-event/v1/examples/review-changes-requested.json

jira-poll-adapter.mdSpecify Jira bot-role mapping +2/-1

Specify Jira bot-role mapping

• Adds provider lookup for verified Jira bot identities and limits Jira project permission roles to humans.

docs/normative/normalized-event/v1/jira-poll-adapter.md

Other (1) +34 / -5
normalized-event.schema.jsonRequire distinct bot and human role fields +34/-5

Require distinct bot and human role fields

• Requires bot_role, permits nullable role fields, and conditionally enforces null permission roles for bots and null bot roles for humans.

docs/normative/normalized-event/v1/normalized-event.schema.json

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:58 PM UTC · Completed 1:27 PM UTC

Commit: b977b68 · View workflow run →

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

@qodo-code-review

qodo-code-review Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Bot examples fail JSON replay ✓ Resolved
Description
The updated bot examples set actor.role to null, but normevent.ParseJSON still rejects every
event whose parsed role is empty. Loading either example through the JSON input driver fails
validation before dispatch.
Code

docs/normative/normalized-event/v1/examples/ready-to-code-labeled.json[R18-19]

+    "role": null,
+    "bot_role": "triage",
Relevance

●●● Strong

Fixture/schema alignment findings are accepted when examples contradict parser or normalized-event
validation behavior.

PR-#2650

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed example supplies a null role. The JSON driver calls ParseJSON, while the Go Actor has a
string role and Validate rejects its empty value; the new bot_role field is not represented in that
struct.

docs/normative/normalized-event/v1/examples/ready-to-code-labeled.json[15-21]
internal/harnessdispatch/input/json.go[10-30]
internal/normevent/event.go[118-124]
internal/normevent/event.go[179-190]
internal/normevent/event.go[211-225]

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 updated bot examples cannot be replayed through the JSON input driver because Go validation still requires a non-empty permission role for every actor.

## Fix Focus Areas
- docs/normative/normalized-event/v1/examples/ready-to-code-labeled.json[18-19]
- internal/normevent/event.go[118-124]
- internal/normevent/event.go[211-225]
- internal/harnessdispatch/input/json.go[10-30]

## Recommended Fix
Add the nullable permission role and bot-role representation to the Go event model, and validate human and bot actors according to their respective fields. Test JSON replay with the updated bot examples.

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


2. Users see an unavailable bot trigger ✓ Resolved
Description
cel-triggers-reference.md presents event.actor.bot_role as an available CEL field without
marking provider-backed resolution as planned, although normevent.Actor has no such member and
ToMap cannot supply it. A harness filtering on a documented bot role therefore encounters a
missing-key evaluation error, and MatchHarnesses skips that agent.
Code

docs/guides/user/cel-triggers-reference.md[60]

+| `event.actor.bot_role` | string or null | Provider-resolved canonical bot role such as `review`; `null` for humans and unrecognized bots |
Relevance

●●● Strong

Recent documentation reviews accepted fixes for documented CEL fields that are absent or unsafe in
serialized events.

PR-#5532

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The guide lists bot_role as a current CEL field, but the JSON-derived event map comes from an
Actor struct without that member. The serialization path therefore cannot provide the key, and
MatchHarnesses skips agents whose trigger evaluation returns an error.

docs/guides/user/cel-triggers-reference.md[53-60]
internal/normevent/event.go[106-111]
internal/harnessdispatch/enumerate.go[110-123]
docs/guides/user/cel-triggers-reference.md[57-60]
internal/normevent/event.go[118-124]
internal/normevent/event.go[280-291]
internal/harnessdispatch/enumerate.go[109-127]
Skill: writing-user-docs

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 reference documents `event.actor.bot_role` as available even though runtime events cannot supply it, so bot-role triggers fail evaluation and the affected agents are skipped.

## Fix Focus Areas
- docs/guides/user/cel-triggers-reference.md[56-61]
- internal/normevent/event.go[118-124]
- internal/normevent/event.go[280-291]
- internal/harnessdispatch/enumerate.go[109-127]

## Recommended Fix
Separate currently available actor fields from the planned bot-role contract: move the future behavior into a `> **Planned:**` callout linking to issue #7764. Add `bot_role` to the current field table only when producers populate it in the runtime actor and CEL receives it; at that point, add a dispatch test showing that a trigger selecting a resolved bot role evaluates successfully.

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


3. The new decision swaps template sections ✓ Resolved
Description
ADR 0107 places ## Decision before ## Options, reversing their order in
0000-adr-template.md. Readers comparing this new record with the template encounter a different
sequence for the alternatives and the decision they explain.
Code

docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md[103]

+## Options
Relevance

●●● Strong

ADR convention precedents favor placing Options before Decision for traceable rationale and template
consistency.

PR-#2743

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new ADR puts Options after Decision, whereas the repository template puts Options before
Decision.

docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md[44-46]
docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md[103-115]
docs/ADRs/0000-adr-template.md[26-41]
Skill: writing-adrs

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 new ADR reverses the template's Options and Decision section order.

## Fix Focus Areas
- docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md[44-113]

## Recommended Fix
Move the Options section before Decision, retaining the existing content and leaving Consequences after Decision.

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



Remediation recommended

4. Bot events fail the new event schema ✓ Resolved
Description
normalized-event.schema.json requires actor.bot_role on every actor and role: null on bots,
but the Go event type has no bot-role field and production adapters still assign bots permission
roles. When GitHub, GitLab, or Jira output is checked against the revised schema, both human and bot
events fail validation even though the Go dispatch validator accepts them.
Code

docs/normative/normalized-event/v1/normalized-event.schema.json[296]

+      "required": ["id", "kind", "role", "bot_role", "is_entity_author"],
Relevance

●●● Strong

Accepted precedents require schema documentation to match producers and treat added required fields
as contract changes.

PR-#2650

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The schema’s required bot_role field is absent from the Go Actor type, so current producers
cannot emit it for either human or bot actors. The GitHub and Jira adapters construct actors using
permission roles, and the GitLab producer likewise retains that representation; bot output also
conflicts with the schema’s null-role constraint.

Rule 3092464: Keep normalized event documentation in sync with event payload and normevent structure changes
docs/normative/normalized-event/v1/normalized-event.schema.json[293-296]
docs/normative/normalized-event/v1/normalized-event.schema.json[332-340]
internal/normevent/event.go[106-111]
internal/harnessdispatch/input/ghaevent.go[65-100]
internal/poll/convert.go[89-103]
internal/jirapoll/convert.go[12-37]
docs/normative/normalized-event/v1/normalized-event.schema.json[293-350]
internal/normevent/event.go[118-124]
internal/harnessdispatch/input/ghaevent.go[75-107]
internal/jirapoll/convert.go[27-39]

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 v1 schema requires bot identity fields that production normalized-event producers cannot emit, and it disallows the permission roles they still assign to bots.

## Fix Focus Areas
- docs/normative/normalized-event/v1/normalized-event.schema.json[293-350]
- internal/normevent/event.go[118-124]
- internal/harnessdispatch/input/ghaevent.go[75-107]
- internal/jirapoll/convert.go[27-39]
- internal/poll/convert.go[105-110]

## Recommended Fix
Keep current producer output valid under the v1 schema until the Go event type, production adapters, and authorization path migrate. Coordinate the breaking contract in an appropriate versioned schema: add `bot_role` to the event model, emit explicit null `bot_role` for humans, implement trusted bot-role resolution before emitting bots with a null permission role, and validate adapter output against the new schema.

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


5. Existing v1 events become invalid ✓ Resolved
Description
The v1 schema makes actor.bot_role required and rejects the formerly valid non-null permission
role on bot actors. Existing v1 event documents therefore fail the changed schema, although the
normalized-event contract explicitly requires a v2 contract for validation changes that reject
previously valid documents.
Code

docs/normative/normalized-event/v1/normalized-event.schema.json[296]

+      "required": ["id", "kind", "role", "bot_role", "is_entity_author"],
Relevance

●●● Strong

The repository accepts findings identifying v1 validation changes that reject existing documents and
require a version bump.

PR-#2650

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The v1 versioning table says tightened validation that rejects previously valid documents requires
v2. Requiring bot_role and constraining bot role to null rejects old valid v1 human and bot
documents.

docs/normative/normalized-event/v1/README.md[48-59]
docs/normative/normalized-event/v1/normalized-event.schema.json[293-350]

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 required actor field and stricter bot-role invariant invalidate existing v1 documents despite the contract's explicit versioning guarantee.

## Fix Focus Areas
- docs/normative/normalized-event/v1/normalized-event.schema.json[293-350]
- docs/normative/normalized-event/v1/README.md[48-59]

## Recommended Fix
Publish the breaking actor representation in a v2 schema and provide a migration path for producers and consumers. Keep the v1 schema compatible with existing valid events.

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


Grey Divider

Context sources
✅ Compliance rules (platform): 72 rules
✅ Cross-repo context — repo relationships
  Explored: repo: fullsend-ai/.fullsend (sha: 7c163cad) — View relationship
  Explored: repo: fullsend-ai/agents (sha: a526fa38) — View relationship
Review mode: ⚖️ Balanced: This is a security-sensitive authorization and normalized-event contract change spanning multiple normative documents, schemas, adapters, and fixtures, so it warrants a complete single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@github-actions

github-actions Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Site preview

Preview: https://514e142e-site.fullsend-ai.workers.dev

Commit: b0961d8446007ba4955dd32a7afe133c362c7dcd

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Sep 27, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with zero direct runtime blast radius and strong issue alignment, scored moderate due to normative specification updates and high change coupling in core architecture documentation.

Previous run

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with zero direct runtime blast radius and strong issue alignment, scored moderate due to normative specification updates and high change coupling in core architecture documentation.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Documentation-only contract specification with zero direct runtime blast radius and tight issue alignment, scored moderate due to broad normative file count and high change coupling in core architecture documentation.

Previous run (3)

Risk Assessment: moderate (2/5)

Details

Documentation-only contract specification with zero direct runtime blast radius and tight issue alignment, scored moderate due to broad normative file count and high change coupling in core architecture documentation.

Previous run (4)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with zero direct runtime blast radius and strong issue alignment, scored moderate due to broad file count across normative specs and high change coupling in core architecture documentation.

Previous run (5)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with zero direct runtime blast radius and strong issue alignment, scored moderate due to broad file count across normative specs and high change coupling in core architecture documentation.

Previous run (6)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with zero direct runtime blast radius and strong issue alignment, scored moderate due to broad file count across normative specs and high change coupling in core architecture documentation.

Previous run (7)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with zero direct runtime blast radius and strong issue alignment, scored moderate due to broad file count across normative specs and high change coupling in core architecture documentation.

Previous run (8)

Risk Assessment: moderate (2/5)

Details

Documentation-only design contract with low metadata risk and strong issue alignment, elevated to moderate risk by touchpoints on high-churn architectural files with significant co-commit history.

Previous run (9)

Risk Assessment: moderate (2/5)

Details

Documentation-only design contract with low metadata risk and strong issue alignment, elevated to moderate risk by touchpoints on high-churn architectural files with significant co-commit history.

Previous run (10)

Risk Assessment: moderate (2/5)

Details

Docs-only contract specification with medium blast radius (10 files, 365 lines) and elevated git history churn across touched normative architecture files, maintaining a moderate risk score (2) offset by zero protected-path touches, no dependency changes, and active linked issue alignment.

Previous run (11)

Risk Assessment: moderate (2/5)

Details

Docs-only contract specification with medium blast radius (10 files, 351 lines) and elevated git history churn across touched normative architecture files, maintaining a moderate risk score (2) offset by zero protected-path touches, no dependency changes, and active linked issue alignment.

Previous run (12)

Risk Assessment: moderate (2/5)

Details

Docs-only contract specification with a broad 21-file blast radius and moderate git history churn across normative paths, offset by zero protected-path touches, no CI or dependency changes, high rollback safety, and close alignment with an active linked issue.

Previous run (13)

Risk Assessment: moderate (2/5)

Details

Documentation-only normative contract change across 21 files with strong issue alignment (closes #7764); anchored near the prior score of 2 (moderate), unchanged despite substantive content updates since the prior review, since the change remains docs-only with no protected paths, no security-sensitive code paths, and no dependency/CI changes.

Previous run (14)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with zero direct runtime blast radius and strong issue alignment; anchored to the prior review's score of 2 (moderate) since this PR's own content is unchanged since the prior review (only an upstream main-merge commit was added).

Previous run (15)

Risk Assessment: moderate (2/5)

Details

Documentation-only PR with zero direct runtime blast radius and strong issue alignment, scored moderate due to broad file count across normative specs and high change coupling in core architecture documentation.

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review

Findings

Low

  • [backward-incompatible] docs/normative/normalized-event/v1/normalized-event.schema.json:321 — The schema's actor object has additionalProperties: false, so events carrying the new bot_role and role_verified fields fail validation against older vendored copies of the v1 schema. The README now states that Fullsend controls the producers and consumers and that older strict schema copies are unsupported after rollout. That documents the risk but does not remove it. The README also says the Go normevent.Validate() is looser than the schema until the adapter migration reaches parity, so producers and consumers could disagree in the interim.
    Remediation: Gate adapter emission of bot_role and role_verified on the validator and consumer updates, as the README already requires.

The prior stale-doc finding at docs/normative/normalized-event/v1/README.md (examples wording) is not observed in the current diff.

Previous run

Review

Findings

Low

  • [stale-doc] docs/normative/normalized-event/v1/README.md:310 — The Examples section says the examples/adr-0107/ representations are "not emitted until the adapter migration crosses its explicit boundary". The rest of the contract now says the adapter implementation itself is the migration boundary and no per-event mode marker is required. "Explicit boundary" reads like wording left over from the earlier mode-marker design.
    Remediation: Reword lines 308-310 to match the adoption wording used elsewhere, e.g. "The examples/adr-0107/ subdirectory contains target representations and is not emitted until an adapter adopts the ADR 0107 target implementation."

  • [backward-incompatible] docs/normative/normalized-event/v1/normalized-event.schema.json:321 — The v1 schema adds optional actor.bot_role and actor.role_verified while actor keeps additionalProperties: false. A consumer validating against an older pinned v1 schema will reject events from producers that emit the new fields. The new allOf rules also tighten validation. The v1 compatibility text calls the fields additive but never says v1 consumers must tolerate them before producers emit them. The ! in the title is appropriate.
    Remediation: State in the normative README that v1 consumers must tolerate bot_role and role_verified before producers emit them, or add a schema version note.


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)

Review

Findings

Medium

  • [fail-open] docs/normative/authorization/v1/README.md:403 — A configuration or deployment marker decides whether an adapter uses registered-identity authorization or the more permissive legacy behavior. The contract does not say what happens when that marker is absent, empty, malformed, or unavailable. The spec says a missing field must not be taken as a sign of legacy status, and that resolver failure or a no-match result denies non-label bots. Neither rule covers the input that selects the mode. An implementer could fall back to legacy authorization when migrated configuration cannot be read, and unregistered review bots would be let through. This is a gap in the spec, not a confirmed runtime default.
    Remediation: Allow compatibility handling only when legacy mode has been set explicitly by a trusted source. State that dispatch is rejected when the mode configuration is empty, malformed, unavailable, or unverifiable, and that a migrated adapter must never drop back to legacy authorization automatically.

Low

  • [backward-incompatible] docs/normative/normalized-event/v1/normalized-event.schema.json:295 — Old events still validate. The PR adds optional bot_role and role_verified properties, plus conditional allOf rules that only apply when those fields are present or role_verified is false. New events are the problem. actor keeps additionalProperties: false, so a consumer pinned to the old v1 schema will reject any new producer event that carries bot_role or role_verified. External v1 validators would break if producers roll out first. The Go normevent.Actor struct has no BotRole or RoleVerified fields, so the new fields are silently dropped and the Go types no longer match the schema.
    Remediation: State in the README and ADR that adding these fields breaks strict v1 consumers, and that consumers must upgrade before producers emit them. Track the Go struct and validator update as follow-up work.

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)

Review

Findings

Low

  • [backward-incompatible] docs/normative/normalized-event/v1/examples/ready-to-code-labeled.json:18 — The top-level fixtures ready-to-code-labeled.json and review-changes-requested.json were edited in place: bot actor.role changed from write to none, with no bot_role or role_verified. This contradicts docs/normative/normalized-event/v1/README.md (lines 301-304), which says the existing top-level fixtures retain the legacy v1 representation and that target fixtures live under examples/adr-0107/. The edited fixtures are now a hybrid: neither the legacy form (role: write) nor the full target form. Consumers that use them as legacy conformance vectors will see a changed value.
    Remediation: Restore "role": "write" in examples/ready-to-code-labeled.json and examples/review-changes-requested.json so they match the README's legacy claim (the target form already exists under examples/adr-0107/), or update the README to describe what these fixtures now represent.

  • [backward-incompatible] docs/normative/normalized-event/v1/normalized-event.schema.json:295 — The v1 actor schema adds two optional fields, bot_role and role_verified, plus if/then conditionals constraining them for bots. Bots may also now carry role: none (none was already in the role enum on the base branch; what is new is its use for bots). Strict v1 consumers that reject unknown properties may fail on post-migration events, and CEL triggers or consumers that compare actor.role for bot actors may behave differently once an adapter crosses the ADR 0107 migration boundary. The README (lines 188-196) says both representations are valid v1 forms and production adapters do not yet emit the target form, which reduces the risk, but the schema version stays at v1.
    Remediation: Confirm that no downstream consumers validate strictly or match on bot actor.role values. Document that consumers must tolerate bot_role, role_verified, and role: none on bots.


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)

Review

Findings

Low

  • [backward-incompatible] docs/normative/normalized-event/v1/examples/ready-to-code-labeled.json — The top-level fixtures examples/ready-to-code-labeled.json and examples/review-changes-requested.json were changed from bot role: write to role: none. That is target-state output, and the current producer (internal/harnessdispatch/input/ghaevent.go) still sets the role from MapGitHubPermission. The PR's own normalized-event/v1/README.md (lines 297-300) says the existing top-level fixtures retain the legacy v1 representation and that target fixtures live under examples/adr-0107/. The edited fixtures now contradict that statement.
    Remediation: Revert the top-level fixtures to role: write and keep the target form only under examples/adr-0107/. Otherwise update the README statement at lines 297-300 and mark the top-level fixtures as target-state.

  • [backward-incompatible] docs/normative/normalized-event/v1/normalized-event.schema.json:316 — The schema description (line 319) says post-migration producers use none for bots and that legacy bot permission values remain valid in v1 during migration. The README (around lines 188-192) reads as a current requirement: "the compatibility value role: none is retained for bots". The current producer still copies the forge permission. The README does say legacy status is decided by the adapter migration boundary, so this is a wording inconsistency, not a structural break.
    Remediation: In the README, state that role: none for bots is the post-migration target and that pre-migration events may carry a legacy forge permission value, consistent with the schema description.

  • [breaking-schema] docs/normative/normalized-event/v1/normalized-event.schema.json:295 — actor keeps additionalProperties: false and this PR adds the optional bot_role and role_verified. A consumer validating against a pre-PR copy of the schema would reject events from migrated producers that emit them. ADR 0107 calls the fields additive and requires no version change, which holds only for consumers that ignore unknown fields, and neither the README nor the ADR says v1 consumers must. No producer emits the fields yet and the in-tree decode paths do not use DisallowUnknownFields, so this is a documentation-clarity gap.
    Remediation: State in the README or ADR 0107's compatibility note that v1 consumers MUST ignore unknown actor fields, or must update their schema copy before adapters cross the migration boundary.


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)

Review

Findings

Medium

  • [logic-error] docs/normative/authorization/v1/README.md:221 — The label-added exception sets requirements that cannot all hold together. README lines 214-227 (repeated in ADR 0107 rule 2) say the target exception applies for every source system, is authorized regardless of actor.role, and is preserved as current and target behavior. They then require evidence that the label mutation is controlled at or above the minimum platform authority for the stage the label can trigger, or the event is denied. (1) The same README says triage users can apply labels while mutation stages need write, so an accepted forge label mutation alone would not meet the new requirement for a label that triggers a code harness; enforcing it would change GitHub's current behavior rather than preserve it, and the text leaves unclear whether the requirement covers GitHub. (2) The platform gate MUST NOT inspect the label name and harness/CEL owns the label-to-stage mapping, yet the contract never says how the gate learns which stage a label can trigger, so it cannot select the threshold.
    Remediation: Define one consistent label authorization algorithm: either specify how the platform obtains the candidate harness's stage threshold and verifies sufficient authority when the forge's label permission is weaker, or define label application as a separate authorization grant that does not imply the ordinary stage threshold. State explicitly whether the requirement applies to GitHub, and align ADR 0107 rule 2 and the preservation claims with that decision.

Low

  • [pattern-inconsistency] docs/normative/normalized-event/v1/normalized-event.schema.json:322 — actor.properties.bot_role sets minLength: 1 but no maxLength. Other string properties in this schema with minLength: 1 pair it with a maxLength (usually 255). The nullable type is intentional and should be kept.
    Remediation: Add "maxLength": 255 to bot_role and keep the nullable type.

Prior findings at jira-poll-adapter.md:74, authorization/v1/README.md:216 and :394, examples/ready-to-code-labeled.json:18, architecture.md:379 and authorization/v1/README.md:9 are not observed in the current diff.


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 (6)

Review

Findings

Medium

  • [logic-error] docs/normative/normalized-event/v1/jira-poll-adapter.md:74 — The new target actor.role mapping forces none for every Jira actor classified as a bot, but the unchanged actor.kind row still classifies an actor as bot from a display-name automation pattern or merely because it is the authenticated poller account. Those heuristics are not the authoritative provider metadata ADR 0107 requires. A human whose display name matches the pattern (or who uses the poller account) would lose their verified Jira project role and be denied on non-label paths when bot-role resolution returns no match.
    Remediation: Separate the legacy Jira heuristics from the post-migration mapping; require authoritative provider bot metadata or an explicitly verified service identity for target bot classification, and keep human classification and project-role authorization otherwise.

  • [fail-open] docs/normative/authorization/v1/README.md:216 — The target label-added exception is generalized to every source system based on an accepted label mutation plus actor-to-transition provenance. That shows who changed a label, not that the actor meets the stage's threshold; in configurable systems such as Jira, label-edit permission can be granted below Fullsend's write threshold. "Equivalent forge-authoritative evidence" does not require equivalent authorization strength, so a future adapter could enable a weaker gate. Mitigated: the exception is stated as implemented only for GitHub and not to be enabled elsewhere until evidence exists, so no runtime path opens now.
    Remediation: State that a non-GitHub adapter may enable the exception only after establishing that label application enforces at least the stage's minimum authority, with absent/unverifiable evidence denying (falling back to human thresholds or registered-bot authorization). Mirror in ADR 0107 rule 2.

  • [architectural-conflict] docs/normative/authorization/v1/README.md:394 — The v1 versioning table still lists "remove a documented exception" as breaking (requires v2), yet the PR narrows the documented "Bot-submitted reviews (GitHub)" exception and, after migration, denies unregistered non-label bots (ADR 0107 Consequences concede existing CEL routes stop matching) via a self-declared "ADR 0107 migration exception" within v1, with no concrete migration-boundary signal or deprecation window for external harness authors. Mitigating: ADR 0015 lets an ADR state when to bump major versions, and ADR 0107 does so explicitly. The table and the exception paragraph are now in tension.
    Remediation: Amend the v1 versioning table to list this security tightening explicitly (or place it under authorization/v2), and publish a migration-boundary signal plus a deprecation window for CEL authors relying on third-party bot routing.

  • [architectural-conflict] docs/normative/normalized-event/v1/examples/ready-to-code-labeled.json:18 — The canonical v1 examples (ready-to-code-labeled.json, review-changes-requested.json) were rewritten to the unimplemented ADR 0107 target shape (role: none, bot_role, role_verified: true, replacing role: write), while production adapters still emit the legacy shape. In-repo tests loading these fixtures do not assert on actor role, but external consumers copying the normative examples get a representation the current runtime does not produce.
    Remediation: Keep legacy-shaped examples under v1/examples/ and add target-shape examples separately, or annotate the changed examples as post-migration output.

Low

  • [permission-expansion] docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md:79 — Once recognized, a bot is exempt from observation and mutation thresholds on all non-label transitions, including schedule/manual. A registered observation-role bot can become an eligible trigger for a mutation harness without write-level permission; registration acts as a grant of dispatch authority. The ADR authorizes this explicitly; no runtime change is implemented.
    Remediation: Consider a provider-controlled, target-scoped bot-to-stage grant, or make the mutation-dispatch consequences explicit in registration/revocation requirements.

  • [pattern-inconsistency] docs/normative/normalized-event/v1/normalized-event.schema.json:322 — bot_role uses a nullable union type and "const": null, whereas elsewhere in this schema optional fields use a single type and inapplicable fields are disallowed via "<property>": false. Documented consistently across the ADR/READMEs; style only.

  • [breaking-schema] docs/normative/normalized-event/v1/normalized-event.schema.json:316 — actor has additionalProperties: false, so out-of-repo consumers validating against a previously vendored v1 schema would reject events carrying bot_role/role_verified until refreshed. Adding optional fields is classed non-breaking and no adapter emits them yet, but there is no rollout note for strict validators.

  • [backward-incompatible] docs/normative/normalized-event/v1/normalized-event.schema.json:341 — The new allOf rules only trigger on the new fields, so existing v1 events stay valid. The authorization table says such events "fail NormalizedEvent validation", but Go normevent.Validate() does not implement these rules, so schema and Go validator diverge until the Go side migrates.

  • [doc-style] docs/architecture.md:379 — Line is ~93 characters while the surrounding rewrapped prose is wrapped at 80.

  • [doc-style] docs/normative/authorization/v1/README.md:9 — Line is ~95 characters while the surrounding intro prose is wrapped at 80.


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)

Review

Findings

Medium

  • [contract-inconsistency] docs/normative/authorization/v1/README.md:129 — The target contract gives conflicting outcomes for a human-added label event whose permission lookup fails. Lines 123–127 say role_verified: false denies the event for humans regardless of the role string. The next sentences say role_verified is not an authorization input on the permanent GitHub label-exception path. That sentence is not limited to bots, so an implementation can deny or admit the same human label event depending on which passage it follows.
    Remediation: Define the human label-added case explicitly. Either carve the label exception out of the human fail-closed rule and table row, or limit the "not an authorization input" statement to bot actors and require successful permission verification for human label events.

Low

  • [architectural-coherence] docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md:23 — ADR 0107 extends the Authorization Contract v1 and NormalizedEvent v1 specs but calls them "the existing v1 contracts" without linking to their docs/normative/ paths. ADR 0015 requires ADRs that introduce or change normative contracts to link the normative tree path.
    Remediation: Link docs/normative/authorization/v1/ and docs/normative/normalized-event/v1/ from ADR 0107.

  • [contract-inconsistency] docs/normative/normalized-event/v1/normalized-event.schema.json:319 — The authorization README says actor.role MUST be none for actor.kind: bot (ADR 0107 target). The schema still accepts legacy permission values for bots. The PR documents this as intentional migration tolerance, but the MUST is not scoped to post-migration producers in the same sentence.
    Remediation: Scope the "MUST be none" sentence to post-migration (ADR 0107) producers. Add a kind: bot → role: none schema conditional once adapters migrate.


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 (8)

Review

Findings

Medium

  • [logic-error] docs/normative/authorization/v1/README.md:180 — The new row Custom repository roles (GitHub) | Mapped to none; denied until custom roles are handled platform-wide denies every custom repository role. This contradicts the permission-mapping algorithm earlier in the same contract (around lines 55-75). That algorithm resolves a custom role from the strongest base-permission flag in user.permissions (e.g. push → write) and falls back to the legacy permission field. The preceding row (line 176) already denies only custom roles with no effective permission signal. Following the new normative row would reject supported custom-role actors.
    Remediation: Remove the unconditional custom-role row, or limit it to custom roles with no usable permission signal so it matches the mapping algorithm and the row at line 176.

Low

  • [doc-style] docs/normative/authorization/v1/README.md:117 — actor.role-keyed is missing backticks around the actor.role field identifier. Every other field reference in the paragraph uses code font, and ADR 0107 writes it as `actor.role`-keyed.
    Remediation: Write `actor.role`-keyed.

  • [consumer-compatibility] docs/normative/normalized-event/v1/normalized-event.schema.json:321 — The Go normevent.Actor struct (internal/normevent/event.go) has no BotRole or RoleVerified fields. Schema-valid events that carry the new fields will decode without error, and the fields will be dropped. The docs describe these fields as target-state, so this is informational.
    Remediation: Add the fields to the Go struct when adapters adopt ADR 0107, and make sure no consumer treats a missing bot_role as a deny signal on the legacy path.


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 (9)

Review

Findings

Medium

  • [api-contract] docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md:24 — The ADR says no new authorization version is required, but its target policy replaces the unconditional GitHub bot-review exception with registered bot-role recognition. Authorization Contract v1's Versioning section explicitly requires v2 when removing a documented exception. Deferring implementation does not resolve this migration-policy contradiction.
    Remediation: Either version the authorization change as v2 or explicitly amend the versioning policy in the ADR and normative contract, explaining the migration boundary. The additive normalized-event fields can remain in v1 independently.

  • [logic-error] docs/normative/normalized-event/v1/README.md:112 — The schedule/manual target contract includes workflow GITHUB_ACTOR, but requires authoritative metadata to classify that identity as a bot and then resolve its registered bot role. GITHUB_ACTOR can identify a human operator, leaving human-initiated manual dispatch without a conforming authorization path despite ADR 0107 preserving human permission-based authorization. This concerns the target contract, not an implemented runtime regression.
    Remediation: Classify the actual identity: humans use verified permission thresholds and bots use registered bot-role resolution. Alternatively, require an explicitly configured bot service identity and remove GITHUB_ACTOR as an interchangeable example.

Low

  • [doc-style] docs/architecture.md:389 — The authorization list item ends with two consecutive parenthetical citation sentences rather than the surrounding semicolon-separated multi-document style.
    Remediation: Combine ADR 0107, Authorization Contract v1, and ADR 0054 into one parenthetical citation block.

  • [doc-style] docs/normative/authorization/v1/README.md:109 — actor.role lacks inline code formatting in the phrase actor.role-keyed, unlike adjacent field references and ADR 0107.
    Remediation: Write it as actor.role-keyed.

  • [doc-style] docs/guides/user/cel-triggers-reference.md:60 — The new actor-field rows place optionality in the Type column, whereas existing rows describe conditional presence in the Description column.
    Remediation: Keep the types as string or null and boolean, and move optionality into their descriptions.

  • [api-shape] docs/normative/normalized-event/v1/normalized-event.schema.json:344 — The human-actor conditional redundantly declares role as a string; the shared property definition already constrains it to an enumerated string. This adds no validation constraint.
    Remediation: Remove the redundant role declaration from the human-actor then.properties block.


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 (10)

Review

Reason: time-budget

This PR was NOT reviewed. Do not count this as an approval.

Previous run (11)

Review

Findings

Medium

  • [logic-error] docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md:101 — Decision rule 1 says a recognized bot only satisfies the bot-identity prerequisite and that "the selected harness's generic transition/target policy and any applicable observation-versus-mutation checks remain mandatory before dispatch." Default thresholds in Authorization Contract v1 (observation=triage+, mutation=write+) are keyed on actor.role, which this same ADR forces to none for every bot. none cannot satisfy triage+ or write+, so if those thresholds apply uniformly, no recognized bot could ever dispatch — contradicting the bot dispatch gate ("only a non-null, provider-resolved actor.bot_role can pass"). The contract never states which of two readings is intended: (a) recognized bots are exempt from actor.role-keyed Default thresholds, with remaining controls being bot_role recognition plus harness CEL/transition-target policy, or (b) each registered bot_role is bound at the platform gate to observation-vs-mutation classes. Rule 1's "without a bot-name-to-stage allowlist" leans toward (a), but "checks remain mandatory" and the claim that CEL must not become the authorization boundary lean toward (b), leaving the target contract unimplementable as written.
    Remediation: Resolve the contradiction explicitly in ADR 0107 Decision rule 1 and mirror it in authorization/v1's Default thresholds / Bot identity fields sections. Pick one: (a) after successful bot_role recognition, do not apply actor.role-keyed observation/mutation thresholds to bots (authorization is recognition plus harness CEL/transition-target policy; qualify or remove "observation-versus-mutation checks remain mandatory"), or (b) bind each registered bot_role to the event/stage classes it may authorize and enforce that binding at the platform gate.

  • [fail-open] docs/normative/authorization/v1/README.md:186 — The Label application (GitHub) exception is still written as unqualified current policy ("the event is authorized regardless of actor.role") plus a one-line compatibility callout. Unlike the adjacent Bot-submitted reviews exception, rewritten in this PR into an explicit current-compatibility-vs-ADR-0107-target pair, the label exception has no target-behavior paragraph. ADR 0107 Decision rule 2 says a label transition "is not by itself sufficient authorization evidence," and the fail-closed table denies unrecognized/failed bot-role lookups — but the living Exceptions section, which implementations follow, never states that replacement for labels. As written, an ADR 0107 implementation could leave the label exception exactly as-is, letting an unrecognized or unrelated bot trigger dispatch via a label add. internal/harnessdispatch/auth.go's IsAuthorized() confirms today's compatibility text matches production (any GitHub label-added event is authorized regardless of actor) — the gap is the missing target-behavior statement.
    Remediation: Mirror the review-exception rewrite for the label exception: state current compatibility (any GitHub label-added event is authorized today) and the ADR 0107 target (bot-originated label-added requires a provider-resolved actor.bot_role; human label-adders continue via GitHub's implicit triage-level permission). Scope the fail-open compatibility behavior to expire once the resolver lands.

Low

  • [architectural-conflict] docs/normative/authorization/v1/README.md:222 — The Schedule and manual dispatch section states only the ADR 0107 target in body text (adapters resolve actor.bot_role, leave actor.role as none) plus a callout that existing compatibility handling remains authoritative until the resolver is wired in — but never states what that current compatibility behavior actually is. The adjacent Bot-submitted reviews section, edited in the same PR, labels current vs. target explicitly. docs/normative/normalized-event/v1/README.md (also modified by this PR) still documents that adapters currently set actor.role to the service identity's effective repository permission (typically write) so the standard write+ gate applies, but this statement was not carried into the authorization contract's own Schedule/manual section.
    Remediation: Add an explicit "Current compatibility behavior" paragraph under Schedule and manual dispatch, matching the pattern used for Bot-submitted reviews: state that adapters currently set actor.role to the service identity's effective repository permission (typically write), satisfying the standard authorization gate, until provider-backed bot-role resolution replaces this.

  • [architectural-conflict] docs/normative/authorization/v1/README.md:159 — The fail-closed table row reads "actor.role is empty or missing for a human | Event fails NormalizedEvent validation; never reaches dispatch," qualified to apply "for a human" only. normalized-event.schema.json requires actor.role unconditionally for every actor (required: ["id", "kind", "role", "is_entity_author"], with no kind-based exception), and the role enum does not accept an empty string or null. A bot event with an empty/missing actor.role fails schema validation the same way a human one does, so the "for a human" qualifier is inaccurate and could mislead a reader into thinking bot events may omit actor.role.
    Remediation: Remove the "for a human" qualifier from this row so it reads "actor.role is empty or missing", since this field is required for all actors regardless of kind in the v1 schema.

  • [markdown-formatting] docs/guides/user/cel-triggers-reference.md:65 — A stray blank line remains between the event.state.change_proposal row and the event.state.conversation row in the NormalizedEvent field reference table — left over from moving the ADR 0107 blockquote callout to after the table in this revision. In GFM/CommonMark, a blank line terminates table parsing, so event.state.conversation and the following event.source.system row render as an orphaned, headerless text fragment instead of continuing the table.
    Remediation: Delete the blank line between the event.state.change_proposal and event.state.conversation rows so the table remains contiguous.


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 (12)

Review

Findings

Medium

  • [logic-error] docs/guides/user/cel-triggers-reference.md:59 — The NormalizedEvent field table states as current CEL semantics that event.actor.role is "always none for bots." Production adapters do not do that: internal/harnessdispatch/input/ghaevent.go assigns ev.Actor.Role from the GitHub collaborator-permission lookup for every actor (no kind guard before the assignment), and internal/jirapoll/convert.go's resolveRole maps Jira project membership with no branch on actor kind either — so a bot with collaborator or project-role access gets a non-none role today. The mid-table blockquote added this round only qualifies bot_role/role_verified as unimplemented ADR 0107 fields; it does not qualify the actor.role claim, and unlike the normative NormalizedEvent README this user guide has no document-level "target contract" banner. A trigger author who filters on actor.role == "none" as a bot signal will not get the behavior this table describes.
    Remediation: Describe today's actor.role mapping for bots (collaborator/project permission when lookup succeeds, else none) and mark "always none for bots" as ADR 0107 target behavior in the same note that already qualifies bot_role/role_verified.

  • [architectural-conflict] docs/normative/authorization/v1/README.md:161 — The fail-closed table states that a bot whose actor.role is not none causes the "Event [to fail] NormalizedEvent validation; never reaches dispatch." This PR's own normalized-event.schema.json says the opposite: the actor.role description explicitly preserves "legacy bot permission values remain valid in v1," and the new allOf conditionals constrain bot_role/role_verified but never require role: "none" when kind is bot. A document with kind: bot, bot_role: "review", role_verified: true, and role: "write" validates against this PR's shipped schema. The table is presented as ADR 0107 target behavior under a document-level "not yet implemented" banner, but it still names NormalizedEvent schema validation — the actual machine-checkable, potentially cross-repo-consumed artifact — as the enforcement mechanism, which is not accurate for the schema as shipped.
    Remediation: Update the table row so it does not claim the current v1 schema rejects legacy bot roles; describe the (currently undefined) authorization/dispatch outcome for such events during migration instead. When ADR 0107-aware producers begin emitting bot_role/role_verified, add an allOf requiring role: "none" for bot-kind actors — noting that tightening the schema then would itself be a v1 breaking change per the NormalizedEvent versioning table.

  • [architectural-conflict] docs/normative/authorization/v1/README.md:198 — The "Bot-submitted reviews (GitHub)" section is now written entirely in future tense ("After the resolver is implemented, a GitHub review event is authorized only when...") and no longer documents the currently-authoritative compatibility behavior at all. The adjacent "Label application (GitHub)" section, in contrast, retains its current-behavior description plus an explicit "Current compatibility behavior" note. internal/harnessdispatch/auth.go's IsAuthorized() still authorizes any GitHub review_submitted event when actor.kind is bot, with no role check — that current behavior is no longer documented anywhere in this living contract. Silently dropping a documented exception's current-state description is also in tension with this same document's Versioning table (which classifies "remove a documented exception" as Breaking, requiring v2), while ADR 0107 asserts no new version is required.
    Remediation: Restore a "Current compatibility behavior" statement for GitHub bot-submitted reviews (the collaborator-permission-independent exception still implemented in IsAuthorized()), matching the pattern used for Label application, before or alongside the target-contract description.

  • [permission-expansion] docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md:99 — Decision rule 1 lets any recognized bot_role satisfy the platform dispatch identity gate for whatever event/stage that identity emits, with no observation-vs-mutation (or per-role stage) threshold applied at the platform gate itself for bots — the "does not grant forge permission, mutation authority, or a hardcoded destination stage" clause describes the intended routing split, not a second platform-level check. Pre-contract IsAuthorized() is narrower on event type (write+, except the GitHub label-added and bot review-submitted exceptions). Under the new rule, a registered narrow-purpose role (e.g. triage) would pass the platform gate for a mutation-class event it happens to emit, leaving CEL/harness routing as the sole remaining least-privilege control between that identity and the action.
    Remediation: Bind each registered bot_role to the event/stage classes it may authorize, or apply the existing observation-vs-mutation thresholds at the platform gate for bots (not solely via CEL/harness routing). Keep CEL as defense-in-depth, not the sole least-privilege control for bot dispatch scope.

Low

  • [logic-error] docs/normative/normalized-event/v1/README.md:108 — The schedule/manual authorization paragraph still calls those sources "trusted operator actions, not end-user webhook events," while the newly added sentences in the same paragraph say authorization comes from recognized bot identity via the same identity lookup used on webhook paths. Unlike authorization/v1/README.md's own "Schedule and manual dispatch" section — which explicitly states an unrecognized or unresolved service identity is denied — this paragraph never says so. This document does carry a top-of-file ADR 0107 "not yet implemented" banner, which softens the concern, but the specific deny-on-unresolved statement present in the sibling document is still missing here.
    Remediation: Align this paragraph with authorization/v1/README.md's "Schedule and manual dispatch" section by stating explicitly that an unrecognized/unresolved service identity is denied.

  • [logic-error] docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md:108 — Decision rule 2's second sentence ("Bot classification without a recognized role, a label transition, or a bot-authored review is not by itself sufficient authorization evidence") is ambiguously worded: "X without A, B, or C" can be misread as "X that lacks A, B, and C," which would suggest a label transition or bot-authored review could compensate for a missing recognized role. The surrounding text (the same rule's first sentence, and rule 1's "MUST NOT broaden authorization to an unrecognized bot") already forbids that reading, so this is a wording/clarity risk in a normative decision rule rather than an actual contradiction.
    Remediation: Rewrite as a list of independently-insufficient signals, e.g. "Neither bot classification without a recognized role, nor a label transition, nor a bot-authored review is by itself sufficient authorization evidence."

  • [architectural-conflict] docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md:44 — The statement "This is an ADR-only change. It specifies the target contract and does not yet modify the normalized-event structs, forge adapters, resolver, or dispatch authorization implementation" is accurate about the Go runtime but understates the PR's actual scope: this PR also modifies the normative NormalizedEvent schema, the authorization contract README, the CEL triggers reference guide, the jira-poll-adapter spec, and two example fixtures — all outside the ADR file itself.
    Remediation: Reword to scope the claim to the implementation, e.g. "This decision does not yet modify the Go runtime implementation (normalized-event structs, forge adapters, resolver, or dispatch authorization); accompanying normative specification documents are updated in this PR to describe the target contract."

  • [markdown-formatting] docs/guides/user/cel-triggers-reference.md:66 — A blockquote callout about actor.bot_role and actor.role_verified is inserted directly inside the NormalizedEvent field reference Markdown table, between the event.state.change_proposal row and the event.state.conversation row. A blank line before a blockquote terminates GFM table parsing, so the two rows after the callout (event.state.conversation, event.source.system) render as an orphaned table fragment without headers.
    Remediation: Move the blockquote note to after the complete table (after the event.source.system row) instead of inserting it mid-table.


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 (13)

Review

Findings

High

  • [logic-error] internal/jirapoll/convert_test.go:768 — Adding "bot_role": null to docs/normative/normalized-event/v1/examples/jira-fs-triage-comment.json breaks TestToNormalizedEvent_JSONRoundTrip. That test marshals Poller.toNormalizedEvent() output and reflect.DeepEquals the resulting generic maps against the fixture. internal/normevent.Actor has only ID, Kind, Role, IsEntityAuthor — no BotRole field — so the Go-produced map has no bot_role key, while the fixture map now has bot_role: nil. Maps with different key sets are never DeepEqual, so this test fails as soon as this PR merges (lint.yml runs go test -race ./... on every PR). Confirmed unresolved from the prior review round; no .go files are touched by this PR. (TestToNormalizedEvent_MatchesFixture, which unmarshals into a typed Event, would still pass — this is specifically the map-based round-trip test.)
    Remediation: Do not add a bot_role key to fixtures consumed by exact JSON-map round-trip tests until internal/normevent.Actor gains a BotRole field and adapters emit it. Omit bot_role from jira-fs-triage-comment.json specifically (or implement the Go-side field in this PR).

  • [incorrect-doc] docs/normative/authorization/v1/README.md:91 — The living authorization contract, NormalizedEvent v1 README, CEL reference, Jira adapter spec, and JSON Schema present ADR 0107's bot-identity rules as current MUST behavior (bots always have role: "none"; only a non-null provider-resolved actor.bot_role can pass the bot dispatch gate; non-conforming combinations "fail NormalizedEvent validation; never reach dispatch"; event.actor.bot_role/role_verified are usable CEL fields). None of this is implemented: internal/normevent.Actor has no BotRole or RoleVerified field, Validate() only requires a non-empty actor.role without branching on kind, ToMap() cannot emit bot_role/role_verified, and IsAuthorized() (internal/harnessdispatch/auth.go) still authorizes GitHub label-added and bot review_submitted without inspecting bot roles, else applies write+ on actor.role. ADR 0107's own Context section calls this an ADR-only target contract; ADR 0054's new note and architecture.md's "migrating" wording are the only implementation caveats present anywhere in this PR — the living contract's "Bot identity fields" section, fail-closed table, the rewritten "Schedule and manual dispatch" exception, the schema description, and the CEL field table carry none. Independently, the unchanged "Label application (GitHub)" exception still authorizes every github/label_changed/added event "regardless of actor.role" with no actor.bot_role mention, contradicting the adjacent unqualified claim that only a non-null, provider-resolved actor.bot_role can pass the bot dispatch gate. Only the "Bot-submitted reviews" exception was reworded this round with "After the resolver is implemented..."; the label exception and the schedule/manual exception were not, so both read as unqualified current policy that conflicts with the new gate.
    Remediation: Mirror the ADR 0054/ADR 0098 caveat everywhere this PR presents bot_role/role_verified/role: "none"-for-bots as live: mark "Bot identity fields", the fail-closed table rows, "Schedule and manual dispatch", the NormalizedEvent v1 actor mapping, the CEL field table, and the jira-poll-adapter actor mapping as the ADR 0107 target contract, not yet enforced by fullsend dispatch, its adapters, or internal/harnessdispatch. Also rewrite the "Label application (GitHub)" exception with the same "until the resolver is implemented, this remains current compatibility behavior" qualifier used for the review exception, so it cannot be read as simultaneous, unqualified current policy alongside the new bot-identity gate.

  • [architectural-conflict] docs/normative/normalized-event/v1/normalized-event.schema.json:338 — The actor schema's allOf constraint still requires role to be the exact value "none" whenever kind is "bot" (bot_role itself is no longer in the required array this round, but this constraint is unchanged in substance). This rejects previously-valid v1 documents: the pre-PR schema had no kind-based restriction on role, and the pre-PR ready-to-code-labeled.json/review-changes-requested.json fixtures themselves used {"kind": "bot", "role": "write"} — exactly the shape the new constraint now rejects. Tracing JSON Schema semantics confirms a minimal pre-PR-valid bot document ({"id": "x[bot]", "kind": "bot", "role": "write", "is_entity_author": false}) now fails schema validation. The NormalizedEvent v1 README's own Versioning section classifies "tighten patterns that reject previously valid documents" as Breaking, requiring a v2 directory; the Authorization v1 README's Versioning table separately lists "remove a documented exception" as Breaking, yet this PR narrows the "Bot-submitted reviews (GitHub)" exception in place. ADR 0107 asserts the opposite ("It does not require a future normalized-event or authorization version") and its Context section claims this PR "does not yet modify the normalized-event structs, forge adapters, resolver, or dispatch authorization implementation" — yet the published v1 JSON Schema itself is mutated in place. Neither v1 README's Versioning table is amended to record an intentional in-place exception. Any external system that validates already-produced or newly-emitted NormalizedEvent v1 documents against this schema (downstream consumers, replay/audit tooling, adapters not yet updated to always emit role: "none" for bots) will see previously-conformant bot-actor documents start failing validation with no v1.x migration path.
    Remediation: Either publish the tightened actor constraints under docs/normative/normalized-event/v2/ (and any narrowed authorization exceptions under authorization/v2/) per ADR 0015, or amend both v1 README Versioning tables and ADR 0107 to explicitly record that this in-place breaking revision is intentional, with a documented compatibility/migration statement for existing v1 consumers and confirmation that no producer still emits a non-"none" role for bots.

Medium

  • [fail-open] docs/normative/authorization/v1/README.md:103 — ADR 0107 states that an unavailable or unverifiable actor-resolution result is authorization-unusable and must not trigger an agent, but the living contract only implements that for bots (role_verified: false implies bot_role must be null, hence denied). For actor.kind: human, the doc only says actor.role contains the forge permission "when actor.role_verified is true" — it never states what happens when a human's role_verified is false while actor.role is still populated (e.g., write). The Fail-closed table has no row for "human actor.role_verified is false" alongside a non-none actor.role, leaving an unspecified case where a human with an unverified/untrusted role value could still satisfy the write+ threshold.
    Remediation: Add an explicit fail-closed table row: human actor.role_verified === false is denied regardless of actor.role. Clarify that omitted role_verified on legacy/pre-migration events is treated as verified (authorize from actor.role as today) only until adapters begin emitting the flag.

  • [permission-expansion] docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md:87 — ADR 0107 Decision rule 1 authorizes any recognized bot "at the platform dispatch gate for events it emits" and states that harness CEL and other routing layers MAY narrow this further but MUST NOT broaden it — with no observation-vs-mutation stage threshold applied to bots at the platform gate itself. Pre-PR documented/implemented bot dispatch was narrower: write+ via installation permission, plus specific exceptions (GitHub label-added, GitHub bot reviews). Under the new rule, a registered bot role (e.g., triage) would satisfy the platform gate for any event type it emits (opened PRs, arbitrary comments, schedule/manual dispatch, etc.) with only CEL — which the authorization contract elsewhere describes as a routing/narrowing mechanism, not an authorization boundary — standing between a recognized-but-narrow-purpose bot identity and a mutation-class action.
    Remediation: Qualify Decision rule 1 so a recognized bot_role satisfies the platform gate only for the event/stage classes that role is registered for, or apply the existing observation-vs-mutation thresholds at the platform gate for bots (not solely via CEL). Keep CEL as defense-in-depth, not the sole least-privilege control for bot dispatch scope.

Low

  • [fail-open] docs/guides/user/cel-triggers-reference.md:61 — event.actor.role_verified is documented as true for bots "when bot-role lookup completed, including a successful no-match result" — i.e., true even for an unrecognized bot. The field name reads as a trust/verification signal, and the CEL reference gives no warning against using it as an authorization check. A harness author skimming the field table could plausibly write a CEL trigger keyed on role_verified == true believing it denotes a trusted/recognized actor, when it only denotes that a lookup attempt completed.
    Remediation: Add a note to the role_verified row (or nearby) stating it is not authorization evidence by itself — for bots it only indicates lookup completion, not recognition — and that triggers narrowing bot access must use actor.bot_role (with has()), not role_verified.

  • [pattern-inconsistency] docs/ADRs/0107-bot-identity-resolution-for-dispatch-authorization.md:116 — ## Options still appears after ## Decision, contrary to the repo's ADR template (docs/ADRs/0000-adr-template.md) and most sibling ADRs. Unfixed since the prior review round.
    Remediation: Move ## Options (and its accompanying text) to appear before ## Decision.

  • [api-shape] docs/normative/normalized-event/v1/jira-poll-adapter.md:70 — The "Actor mapping" table still lists actor.bot_role before actor.role, inconsistent with the canonical id, kind, role, bot_role, is_entity_author ordering used in the schema and elsewhere in this PR. Unfixed since the prior review round.
    Remediation: Swap the two table rows so actor.role precedes actor.bot_role.


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.

fullsend-ai-review[bot]

This comment was marked as outdated.

Bring the PR branch up to date with the latest main changes.

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

fullsend-ai-review Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:48 PM UTC · Completed 10:07 PM UTC

Commit: 1f035ae · View workflow run →

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

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

Keep bot role compatibility in normalized-event v1 while making provider-resolved bot_role optional. Apply PR 7080's fail-closed resolver policy and record this as an ADR-only contract update.

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

fullsend-ai-review Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:57 AM UTC · Completed 9:19 AM UTC

Commit: 91d088f · View workflow run →

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

fullsend-ai-review[bot]

This comment was marked as outdated.

Define the additive role_verified actor field and clarify that bot identity verification is carried by bot_role while legacy role remains none.

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

fullsend-ai-review Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 7:05 AM UTC · Ended 7:07 AM UTC

Commit: a845c0f · View workflow run →

@fullsend-ai-review

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

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:42 PM UTC · Completed 12:57 PM UTC

Commit: 6166feb · View workflow run →

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

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

Use the adapter implementation as the legacy-to-resolved boundary and remove the unnecessary mode marker and deprecation window.

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

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

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:57 PM UTC · Completed 2:07 PM UTC

Commit: e0ccae0 · View workflow run →

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

fullsend-ai-review[bot]

This comment was marked as outdated.

Document the supported producer and consumer rollout contract for the additive bot actor fields, and remove stale migration-boundary wording.

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

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

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:15 PM UTC · Completed 2:26 PM UTC

Commit: b0961d8 · View workflow run →

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

@fullsend-ai-review
fullsend-ai-review Bot dismissed stale reviews from themself October 7, 2026 14:25

Superseded by updated review

This branch was successfully deployed

1 active and 1 inactive deployments
site-preview — b0961d84 Deployed Oct 7, 2026 by github-actions[bot]
dev — b0961d84 Deployed Oct 7, 2026 by ifireball via playback #15700
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge risk/moderate PR risk: moderate type/adr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Define a forge-portable bot identity and authorization contract

5 participants