[ifireball]

Summary

• add ADR 0098 adopting one harness CEL predicate over a resolved entity and optional prompting event
• preserve backward compatibility: harnesses without entity sources remain event-only and may rely on the event being present
• allow harness queries to derive handled state from entity activity, such as an existing triage comment, without requiring a separate receipt or checkpoint
• make events a low-latency source of candidates while scheduled polling evaluates durable entity state directly
• define platform/default and harness scheduling as machinery rather than authorization principals; runs use the harness configured identity
• partially supersede ADR 0063 event reconstruction requirement while retaining its poll command, driver architecture, per-repo scope, and coordination decisions
• annotate accepted ADR 0061 to identify only its event-only CEL context as superseded by ADR 0098
• update the living architecture and agent-architecture problem document

Decision

Event-driven dispatch supplies both the resolved entity and prompting event. Scheduled discovery supplies the entity with a null event. Harnesses with entity sources keep one predicate that may inspect either or both contexts; harnesses without entity sources remain event-only.

Harness enablement and platform policy permit scheduled evaluation. Entity history remains untrusted input; trusted input-selection layers determine which actor-originated instructions are authorized and actionable before they control agent behavior. Every run uses the harness configured identity and permissions.

The detailed normalized-entity, handled-state, and query-planning contract remains follow-up normative work. Efficient cross-entity enumeration may require provider-specific indexing, while persistence may use existing entity state or explicit receipts. Singleton scheduled runs without a natural entity remain outside this ADR.

This captures the architecture discussion in #313. Stable entity identity supports the serialized follow-up model in dependent PR #6909 (ADR 0106).

Verification

• make lint

[qodo-code-review]

PR Summary by Qodo

Adopt entity-first harness evaluation architecture

📝 Documentation 🕐 20-40 Minutes

AI Description

• Adopt one harness predicate over resolved entities and optional prompting events.
• Treat events as candidates while polling evaluates durable entity state.
• Supersede event-reconstruction polling and align architecture documentation.

Diagram

graph TD
  WEBHOOK["Forge Webhook"] -->|event candidate| PROVIDER["Entity Provider"] -->|resolved context| PREDICATE["Harness Predicate"] -->|match| AUTH["Authorization Gate"] -->|approved| RUN["Run Coordinator"] -->|records| RECEIPT["Processing Receipt"]
  POLL["Poll Driver"] -->|scheduled candidate| PROVIDER
  RECEIPT -->|handled state| PROVIDER

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Reconstruct events during polling

• ➕ Preserves the existing event-only predicate contract.
• ➕ Minimizes initial harness migration.
• ➖ Requires recovering ordered event history.
• ➖ Can permanently miss work across checkpoint gaps.
• ➖ Consumes provider API capacity and loses source fidelity.

2. Separate event and entity predicates

• ➕ Makes event-driven and scheduled behavior explicit.
• ➕ Allows each path to use a specialized context.
• ➖ Duplicates routing policy across predicates.
• ➖ Can produce inconsistent decisions for the same entity.
• ➖ Increases harness authoring and maintenance effort.

Recommendation: Use the PR's single predicate over a required entity and nullable event. It unifies event-driven and scheduled evaluation while making durable state authoritative; follow-up normative work should tightly define entity history, query planning, receipts, and compatibility migration.

Files changed (5) +133 / -15

Documentation (5) +133 / -15

0061-harness-cel-dispatch.mdMark event-only CEL context as superseded +4/-0

Mark event-only CEL context as superseded

• Adds a status clarification linking ADR 0099 and replacing the event-only predicate context with resolved entity plus optional event evaluation.

docs/ADRs/0061-harness-cel-dispatch.md

0063-polling-based-work-discovery.mdSupersede event-reconstruction polling decision +6/-2

Supersede event-reconstruction polling decision

• Marks ADR 0063 as superseded while documenting that compatible poll commands and drivers remain valid under entity-first evaluation.

docs/ADRs/0063-polling-based-work-discovery.md

0099-entity-first-harness-evaluation.mdDefine entity-first harness evaluation +104/-0

Define entity-first harness evaluation

• Adds the accepted architectural decision for one CEL predicate over a normalized entity and nullable event. It defines discovery, identity, activity provenance, authorization, processing receipts, coordination, consequences, and deferred normative work.

docs/ADRs/0099-entity-first-harness-evaluation.md

architecture.mdAlign dispatch architecture with entity-first evaluation +13/-11

Align dispatch architecture with entity-first evaluation

• Updates the living architecture to distinguish low-latency event candidates from scheduled entity discovery. It also replaces the prior coordination question with the unresolved entity-history, query-planning, and receipt contract.

docs/architecture.md

agent-architecture.mdUpdate agent coordination workflow for resolved entities +6/-2

Update agent coordination workflow for resolved entities

• Reframes triggers as forge events or scheduled entity discovery and updates the review-agent example to evaluate predicates against a resolved PR entity with event context.

docs/problems/agent-architecture.md

[fullsend-ai-review]

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

Commit: 9892078 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.38

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

1. Scheduled authorization has no principal ⊘ Outdated 🐞 Bug ⛨ Security

Description

Scheduled discovery sets event to null and permits predicates based solely on entity state, but
the centralized authorization mechanism requires a specific acting username. The ADR therefore
provides no enforceable authorization rule for state-only scheduled matches, risking either
unauthorized dispatch or rejection of all such work.

Code

docs/ADRs/0099-entity-first-harness-evaluation.md[R64-65]

+Event-driven dispatch resolves the event's entity and supplies both values;
+scheduled discovery supplies the entity with `event` set to null. Events are a

Relevance

●● Moderate

Authorization documentation findings are often accepted, but no close precedent addresses scheduled
state-only principals.

PR-#5328
PR-#1688

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

ADR 0099 allows scheduled predicates to receive no event and to inspect entity state independently,
while its authorization section only states an invariant without identifying the principal to check.
ADR 0054 defines authorization exclusively in terms of has_write_permission(username),
is_authorized(), and is_event_actor_authorized(username), so the new no-event path cannot apply
the required gate as specified.

docs/ADRs/0099-entity-first-harness-evaluation.md[62-67]
docs/ADRs/0099-entity-first-harness-evaluation.md[82-86]
docs/ADRs/0054-require-authorization-on-all-agent-dispatch-paths.md[68-75]
docs/ADRs/0054-require-authorization-on-all-agent-dispatch-paths.md[96-106]

Agent prompt

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

## Issue description
Define the authorization principal and exact platform check for scheduled entity-first evaluations where `event` is null. Specify how state-only predicates and predicates matching multiple historical activities select authorization provenance, while ensuring unauthorized activity cannot authorize execution.

## Issue Context
ADR 0054's centralized gate authorizes a concrete username through the collaborator-permission API. Replacing reconstructed poll events with entity-only scheduled evaluation removes that actor unless the new contract explicitly selects one.

## Fix Focus Areas
- docs/ADRs/0099-entity-first-harness-evaluation.md[62-86]
- docs/ADRs/0054-require-authorization-on-all-agent-dispatch-paths.md[68-75]
- docs/ADRs/0054-require-authorization-on-all-agent-dispatch-paths.md[96-106]

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

---

2. Run coordination adds second decision ⊘ Outdated 📜 Skill insight ⚙ Maintainability

Description

After deciding the entity-first predicate model, ADR 0099 separately decides that runs use
harness-plus-entity identity as their coordination subject. This independent coordination decision
makes the ADR record more than one architectural decision.

Code

docs/ADRs/0099-entity-first-harness-evaluation.md[R88-91]

+Runs use the harness plus stable entity identity as their coordination subject,
+aligning scheduled and event-driven evaluation with the serialized follow-up
+model proposed in [#6909](https://github.com/fullsend-ai/fullsend/pull/6909).
+Singleton scheduled runs without a natural entity are outside this decision.

Relevance

●● Moderate

Coordination details are sometimes accepted as clarifying consequences, making the one-decision
objection subjective.

PR-#2465
PR-#730

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062089 requires each ADR to record exactly one decision. Lines 62-67 establish the
predicate-input decision, while lines 88-91 introduce a separate run-coordination subject and scope
boundary.

docs/ADRs/0099-entity-first-harness-evaluation.md[60-67]
docs/ADRs/0099-entity-first-harness-evaluation.md[88-91]
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
ADR 0099 decides both the harness evaluation context and the coordination subject used for runs.

## Issue Context
The entity-first predicate is the ADR's primary decision. Move the harness-plus-entity run coordination decision and singleton-run scope into a separate ADR, or make this passage explicitly non-normative if it is only a consequence of an existing decision.

## Fix Focus Areas
- docs/ADRs/0099-entity-first-harness-evaluation.md[88-91]

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

---

3. Architecture decision bullet rewritten ✓ Resolved 📜 Skill insight ⚙ Maintainability

Description

The architecture update removes the existing polling decision narrative and replaces it with a
substantially different entity-first description instead of preserving history through a short
additive annotation. This exceeds the required surgical living-document update.

Code

docs/architecture.md[L273-276]

-  operating on a `NormalizedEvent` struct
-  ([ADR 0061](ADRs/0061-harness-cel-dispatch.md)).
-- Per-repo **polling** complements webhook dispatch: `fullsend poll` uses poll
-  input drivers to discover work from remote systems (Jira first), coordinates

Relevance

●●● Strong

Recent documentation precedent accepts surgical architecture updates accompanying ADR changes and
cross-references superseding ADRs.

PR-#2465
PR-#2743

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062104 limits ADR-triggered living-document changes to surgical additions such as
a Decided: line or question annotation. Lines 273-279 from the prior architecture are removed and
replaced by a new multi-line account at lines 273-282.

docs/architecture.md[270-282]
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 accepted ADR update replaces the existing routing and polling decision text rather than applying a minimal annotation and additive decision entry.

## Issue Context
Preserve the prior decision text where possible, annotate it as refined or superseded, and add a concise `Decided:` entry linking ADR 0099. Avoid restructuring the surrounding architecture narrative.

## Fix Focus Areas
- docs/architecture.md[270-282]

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

---

4. ADR 0061 edit unannounced ✗ Dismissed 📘 Rule violation § Compliance

Description

The PR modifies accepted ADR 0061 by adding a supersession annotation, but the PR description
identifies ADR 0063 and ADR 0099 without explicitly identifying ADR 0061. Reviewers therefore are
not directly alerted to every accepted ADR changed by the PR.

Code

docs/ADRs/0061-harness-cel-dispatch.md[R24-26]

+The event-only CEL context is superseded by
+[ADR 0099](0099-entity-first-harness-evaluation.md), which retains one harness
+predicate but evaluates it against a resolved entity and optional event.

Relevance

●●● Strong

Accepted precedent supports annotating modified ADRs and following explicit cross-reference
conventions.

PR-#5798
PR-#6375

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062059 requires the PR description to identify each modified accepted ADR and
summarize its change. The diff adds a substantive supersession annotation to ADR 0061, whereas the
supplied PR description explicitly names only ADR 0063 and the new ADR 0099.

Rule 1062059: Call out edits to accepted ADRs in PR descriptions
docs/ADRs/0061-harness-cel-dispatch.md[21-26]

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 description does not explicitly identify the modification to accepted ADR 0061.

## Issue Context
Add `ADR 0061` or its filename to the PR description and briefly state that its event-only CEL context is annotated as superseded by ADR 0099.

## Fix Focus Areas
- docs/ADRs/0061-harness-cel-dispatch.md[24-26]

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

---

5. Partial supersession mislabeled ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

ADR 0063 is marked wholly Superseded, although ADR 0099 explicitly retains compatible poll-command
and driver machinery and does not replace 0063's per-repo scope, concrete driver choices, or
coordination design. This removes accepted status from still-current decisions and makes references
to those retained details appear obsolete.

Code

docs/ADRs/0063-polling-based-work-discovery.md[3]

+status: Superseded

Relevance

●●● Strong

Recent precedent favors preserving accepted ADR content and using supersession only for genuinely
replaced decisions.

PR-#6769
PR-#2465

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new status labels all of ADR 0063 obsolete, but both its replacement notice and ADR 0099 say the
poll command and drivers remain valid. ADR 0063 additionally decides the supported installation
scope, initial adapters, pipeline composition, and coordination responsibilities, none of which ADR
0099 supersedes with alternative decisions.

docs/ADRs/0063-polling-based-work-discovery.md[23-27]
docs/ADRs/0063-polling-based-work-discovery.md[103-108]
docs/ADRs/0063-polling-based-work-discovery.md[124-140]
docs/ADRs/0099-entity-first-harness-evaluation.md[23-25]
docs/ADRs/0099-entity-first-harness-evaluation.md[69-73]

Agent prompt

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

## Issue description
Change ADR 0063's status to indicate partial supersession and identify precisely which event-reconstruction portions ADR 0099 replaces. Preserve accepted authority for the poll command, driver architecture, scope, and coordination decisions that ADR 0099 retains rather than restates.

## Issue Context
ADR 0099 expressly preserves compatible poll-command and driver machinery, while ADR 0063 contains additional decisions not replaced by ADR 0099.

## Fix Focus Areas
- docs/ADRs/0063-polling-based-work-discovery.md[1-27]
- docs/ADRs/0063-polling-based-work-discovery.md[103-140]
- docs/ADRs/0099-entity-first-harness-evaluation.md[23-25]
- docs/ADRs/0099-entity-first-harness-evaluation.md[69-73]

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

---

6. Problem doc lacks option trade-offs 📘 Rule violation ⌂ Architecture

Description

The modified agent-architecture.md prescribes entity-first evaluation without organizing at least
two approaches into clearly labeled option and trade-off subsections. This prevents the problem
document from preserving alternative solutions for later evaluation.

Code

docs/problems/agent-architecture.md[R148-151]

+- **Forge events and scheduled entity discovery** identify entities whose
+  harness predicates should be evaluated. Each predicate can inspect the
+  resolved entity and an optional prompting event
+  ([ADR 0099](../ADRs/0099-entity-first-harness-evaluation.md)).

Relevance

● Weak

Recent problem-document precedents explicitly rejected adding options and labeled trade-offs when
the document already presents exploratory alternatives.

PR-#5941
PR-#3058

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062035 requires every modified problem document to describe at least two options
and clearly labeled trade-offs for each. The changed interaction model directly describes
entity-first evaluation, while the surrounding section presents the selected repository coordination
model without the required option/trade-off structure.

Rule 1062035: Problem docs must present multiple options with trade-offs, not a single prescribed solution
docs/problems/agent-architecture.md[130-165]

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 modified problem document prescribes entity-first harness evaluation but does not present at least two distinct approaches with clearly labeled trade-offs.

## Issue Context
Problem documents must preserve alternatives rather than present only the selected architecture. Add explicit option subsections and pros/cons or risks for each approach, while retaining the ADR link as the recommendation or decision reference.

## Fix Focus Areas
- docs/problems/agent-architecture.md[130-165]

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

---

7. 0063 body status mismatches ✓ Resolved 📜 Skill insight ≡ Correctness

Description

The frontmatter status is exactly Superseded, but the body Status section contains Superseded by
plus explanatory text. The two status representations therefore are not identical.

Code

docs/ADRs/0063-polling-based-work-discovery.md[R23-26]

+Superseded by
+[ADR 0099](0099-entity-first-harness-evaluation.md), which preserves compatible
+poll-command and driver machinery but evaluates one harness predicate against a
+resolved entity and optional event instead of requiring pollers to reconstruct

Relevance

● Weak

Close precedents rejected requiring bare body status when explanatory supersession text follows the
status.

PR-#2650
PR-#716

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062095 requires the frontmatter status and body Status value to be identical. The
same file has status: Superseded at line 3 but changes the body value to Superseded by ... at
lines 23-27.

docs/ADRs/0063-polling-based-work-discovery.md[1-3]
docs/ADRs/0063-polling-based-work-discovery.md[21-27]
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 ADR frontmatter says `Superseded`, while its body Status section says `Superseded by` and includes the supersession explanation.

## Issue Context
Both status representations must be identical. Keep `Superseded` as the body status and move the backlink and rationale into a separate note outside the status value.

## Fix Focus Areas
- docs/ADRs/0063-polling-based-work-discovery.md[21-27]

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

---

Context sources

✅ Compliance rules (platform): 65 rules

✅ Skills: writing-user-docs, writing-adrs

Review mode: ⚖️ Balanced

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 ↗

[github-actions]

Site preview

Preview: https://7c39b44a-site.fullsend-ai.workers.dev

Commit: b1b82cf47647fef34ea8857f90629cf6ffa3f74d

[fullsend-ai-review]

Risk Assessment: moderate (2/5)

Details

Re-review of a docs/ADR-only PR (9 files, 383 lines, no protected/security/dependency/CI paths, established non-bot author); Tier 1 metadata is unchanged from the prior round, Tier 2 churn is driven by a pre-existing high-traffic shared file (docs/architecture.md), and Tier 3 remains low-to-moderate since the PR is an explicitly-declared architectural companion to issue #313 rather than a direct implementation, with no risk labels and a fully additive, revertible diff — composite score preserves the prior moderate rating of 2.

Previous run

Risk Assessment: moderate (2/5)

Details

Re-review anchoring applies: Tier 1 metadata is unchanged in kind from the prior round (8 files both times, 271+37 vs 285 lines, medium blast radius, no protected/security/dependency/CI paths, established non-bot author), Tier 2 churn/contention/fix-revert signals remain driven by the same pre-existing high-traffic shared docs files (docs/architecture.md), and Tier 3 remains low since the PR is an explicitly declared architectural companion doc for issue #313 rather than a claimed implementation, with no elevated risk labels and a fully additive, revertible docs-only diff — so the composite weighted score preserves the prior moderate score of 2.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Re-review anchoring applies: Tier1 metadata is essentially unchanged from the prior (2/moderate) round (8 files both times, 285 vs 277 lines, medium blast radius, no protected/security/dependency/CI paths, established non-bot author); Tier2 churn/contention signals remain driven by the same pre-existing high-traffic shared docs files; Tier3 remains low since the PR is an explicitly declared architectural prerequisite for issue #313, not a claimed implementation, with no elevated risk labels and a fully additive, revertible docs-only diff — so the prior moderate score of 2 is preserved.

Previous run (3)

Risk Assessment: moderate (2/5)

Details

Re-review anchoring applies: Tier1 metadata is essentially unchanged from the immediately prior (2/moderate) round (8 files both times, 277 vs 271 lines, medium blast radius, no protected/security/dependency/CI paths, established non-bot author) - the only delta is a ~6-line net refinement across 2 of the 8 files. Tier2 churn/fix-revert signals remain driven by the same pre-existing high-churn files. Tier3 remains low: the PR is an explicitly declared architectural prerequisite for issue #313 rather than a claimed fix, carries no risk labels, and the diff is fully additive and revertible docs content. No new signal category was introduced, so the prior moderate score of 2 is preserved.

Previous run (4)

Risk Assessment: moderate (2/5)

Details

Docs/ADR-only change with no protected/security paths, no CI or dependency impact, and no test-ratio concern, but several touched docs show high recent churn, multi-author contention, and frequent fix/revert history, and the linked issue #313 is only loosely related (architectural precursor, not a direct implementation), yielding a moderate composite score of 2.

Previous run (5)

Risk Assessment: moderate (2/5)

Details

Re-review of this PR chain: Tier1 metadata is unchanged from the immediately prior (2/moderate) round (8 files, 271 vs 268 lines, medium blast radius, no protected/security/dependency/CI changes, established non-bot author) — the only delta since the prior review is a net +3 lines to the already-new docs/ADRs/0098-*.md file. Tier2 churn/fix-revert signals remain driven by the same pre-existing high-churn files (docs/architecture.md, normalized-event README, ADR 0054). Tier3 remains low since the PR is an explicitly declared architectural prerequisite for issue #313, not a claimed fix, with no risk labels and a fully additive, revertible docs-only diff. Composite (~1.86) rounds to 2, preserving the prior moderate score with no material change in risk characteristics.

Previous run (6)

Risk Assessment: moderate (2/5)

Details

Tier1 metadata remains in the same scoring bands as the prior 1/low assessment (8 vs 7 files, 268 vs 240 lines, still medium blast radius, no protected/security/dependency/CI changes, established non-bot author); however this round newly touches docs/normative/normalized-event/v1/README.md, a file with above-baseline 30/90-day churn and fix/revert signal not present in the prior round's Tier2 computation, lifting the weighted composite just enough to round to 2 (moderate) rather than 1 (low); Tier3 remains low-risk since the PR is an explicitly declared architectural prerequisite (not a claimed fix) for issue #313 with no risk labels and a fully additive, easily-revertible docs-only diff.

Previous run (7)

Risk Assessment: low (1/5)

Details

Tier 1 signals (7 files, 240 lines, medium blast radius, no protected/security paths, no dependency or CI changes, established non-bot author) are unchanged from the immediately preceding assessment on this same PR chain, anchoring the score to the prior 1/low; architecture.md persistent high churn/author-contention is a pre-existing baseline already discounted in prior rounds, and Tier 3 scope mismatch is an intentional, declared prerequisite relationship rather than a coverage gap.

Previous run (8)

Risk Assessment: low (1/5)

Details

Docs-only PR adding ADR 0098 and minor edits to existing architecture docs; Tier 1 signals are unchanged from the prior assessment, architecture.md high churn is a persistent baseline not specific to this change, and anchoring rules preserve the prior score of 1.

Previous run (9)

Risk Assessment: low (1/5)

Details

Docs-only PR adding ADR 0106 and minor edits to existing architecture docs; Tier 1 signals are unchanged from the prior assessment, architecture.md high churn is a persistent baseline not specific to this change, and anchoring rules preserve the prior score of 1.

Previous run (10)

Risk Assessment: low (1/5)

Details

Docs-only PR adding ADR 0106 and minor updates to existing architecture docs; Tier 1 signals are essentially identical to the prior assessment (no code, no tests, no CI/dep/protected-path changes, known contributor), and anchoring rules preserve the prior score of 1.

Previous run (11)

Risk Assessment: low (1/5)

Details

Docs-only PR adding ADR 0099 and making minor updates to three existing docs and one existing ADR. No production code, no test changes, no CI or dependency modifications, no protected paths. Author is a known contributor.

[fullsend-ai-review]

Looks good to me

Previous run

Review

Findings

Medium

• [fail-open] docs/normative/authorization/v1/README.md:187 — This round closes the three prior gaps in this dimension: provenance is now bound to a verified, non-user-assertable execution identity (lines 176–181); wildcard eligibility policy is fail-closed without platform-level justification (line 115); a missing/incomplete normalized-entity contract denies entity-first execution (lines 118, 171–174); and identified action-indicating elements get a live actor-permission check with fail-closed rows. The remaining hole is the classifier itself: the fail-closed table's rows only apply once an action-indicating element has already been identified ("that element cannot trigger execution") — there is no row for enumerator failure, an unimplemented classifier, or an entity kind the classifier doesn't cover. Classification is defined by purpose ("treated as a request for a stage to run," "when used for that purpose"), so a failed or incomplete classifier is observationally identical to a successful empty set — the intentional state-only path. If action-indicating enumeration silently fails, unauthorized bodies/comments/reviews/labels remain available to CEL/prompt construction and can still match a mutation harness.
  Remediation: Add a fail-closed row: if action-indicating enumeration fails, is unimplemented, or cannot cover the entity kind, deny that entity's evaluation entirely (do not treat it as an empty set). Consider requiring a platform-closed superset (always consider bodies, comments, reviews, and label applications) rather than a per-harness purpose test, so CEL cannot treat an unclassified field as "not a request." Keep the current empty-after-successful-enumeration outcome as the state-only path.
  See also: [logic-error] finding at this location.

• [logic-error] docs/normative/authorization/v1/README.md:193 — The entity-discovery current-actor gate is specified as a single pre-CEL filter that drops action-indicating elements unless the actor meets "the applicable observation or mutation threshold" (lines 190–193), and a harness CEL predicate "cannot ... be relied upon to identify an element for later authorization" (lines 194–196) — i.e., the filter must run and fully resolve before CEL/harness matching. But observation (triage+) and mutation (write+) thresholds differ per ADR 0054's role table, and which one applies is a property of the harness/stage being evaluated — information not yet available at this pre-CEL stage. A single shared filter-then-CEL pipeline can't apply the correct threshold for every candidate harness: using triage retains comments that a mutation harness could then match without any further gate; using write strips triage-role activity an observation harness is supposed to accept. ADR 0098 ("applicable stage threshold") and docs/architecture.md restate the same underspecification rather than resolving it.
  Remediation: State explicitly that the current-actor gate runs per candidate harness, using that harness's observation-vs-mutation threshold, so CEL for a given harness only ever sees elements already filtered under that harness's threshold. Do not describe a single global filter-then-CEL pipeline unless the text also says the filter itself is harness-scoped.
  See also: [fail-open] finding at this location.

---

Note on prior review continuity: This is a re-review (prior head SHA 692b1d2d06d926b2a0f4f535bb940a0e9f56bfa9). Of the 7 prior findings (1 medium from correctness, 3 medium from security, 1 low from intent-coherence, 2 low from docs-currency), this round's revisions substantively resolved 6 of them: the overlay when/event.source.system gap now requires entity.source.system and instructs authors to migrate overlay expressions (closing the prior correctness finding); the "action-indicating element" definition, provenance binding, wildcard-policy fail-closed rule, and entity-contract fail-closed row are now specified (closing 2 of the 3 prior security findings); the scheduled-discovery attributability tension is now addressed with an explicit carve-out in docs/problems/security-threat-model.md and a cross-referenced Observability bullet in docs/architecture.md (closing the prior intent-coherence finding); and both prior docs-currency gaps (missing ADR 0098 citation/forward-looking qualifier, and the stale architecture.md authorization-gate description) are resolved. One prior security finding (permission-expansion, the broad dispatch-principal question) is confirmed intentional and downgraded to an info-level note below the reporting threshold. This round's correctness and security sub-agents independently identified two new medium-severity specification gaps in the newly-added "Fullsend-originated entity discovery" enforcement text, both above.

---

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

• [logic-error] docs/ADRs/0098-entity-first-harness-evaluation.md:82 — The Decision now says overlay when expressions receive the required entity and nullable event "so source-specific selection can inspect entity state when no prompting event exists," but this overclaims: source identity today lives on event.source.system (per ADR 0088's overlay examples and the shipped ResolveOverlays/EvaluateOverlay environment), not on entity. The current NormalizedEvent entity object exposes only kind/id/url/key/linked_change_proposal; ADR 0098 defers the field-level entity contract and never requires a source-system field on entity. runtime.forge cannot substitute (a Jira-originated run on GitHub Actions has runtime.forge == "github" and event.source.system == "jira"). Existing event.source.system == "jira"-style overlay when expressions therefore still cannot match on a fullsend poll entity-discovery run. This is a partial fix of a finding raised in the prior review round — the underlying gap remains.
  Remediation: Either require the normalized entity to carry a source-system (or equivalent) field overlay when can inspect and state in Consequences that existing event.source.system overlay expressions won't match on the entity-discovery path until rewritten against it, or retract the "inspect entity state" claim and state plainly that overlay source-selection will not function on the entity-discovery path until a later spec defines the entity source axis.

• [permission-expansion] docs/normative/authorization/v1/README.md:176 — This round adds a live-permission requirement for "action-indicating element" actors, fail-closed rows, and a post-CEL selected-element gate, closing the original slash-command/comment-trigger hole flagged in the prior review (previously high severity). Two specification gaps remain: (1) "action-indicating element" is only exemplified ("a comment containing a slash command"), not defined, so issue bodies, labels, reviews, and other actor-originated state are not clearly covered; (2) the allowed post-CEL strategy — "validating the element selected by CEL" — doesn't fit ADR 0061/0098's boolean harness predicate (CEL doesn't return a selected element), so an implementation could treat "no selected element" as nothing to deny. The Principle (lines 201–203) and Enforcement intro (lines 207–209) also still name trusted invocation provenance alone as the entity-discovery dispatch gate, in tension with the new selected-element MUST.
  Remediation: Define "action-indicating element" beyond a single example. Specify an independent platform enumeration of those elements (pre-CEL removal, or a post-CEL check that doesn't depend on CEL returning an element). Update the Principle and Enforcement intro so provenance is scoped to authorizing enumeration/evaluation, with the live actor-permission check gating action-indicating triggering specifically.

• [fail-open] docs/normative/authorization/v1/README.md:180 — The MUST to "omit or minimize unneeded untrusted fields" is still qualified by "where the normalized entity contract permits" and "retaining ... content ... required by the harness." ADR 0098 still defers the field-level entity contract to follow-up normative work, and the fail-closed table has no row covering a missing or incomplete entity contract. Until that contract exists, "where the contract permits" has no defined boundary, and "required by the harness" lets a harness request injection-bearing fields with no platform ceiling. Unchanged from the prior review round.
  Remediation: Add a fail-closed row: if the versioned normalized-entity contract does not yet enumerate CEL- and prompt-eligible fields, scheduled evaluation is denied (or restricted to a platform-defined allowlist of non-content fields). State that entity-first Execution MUST NOT be enabled before the field-level contract lands.

• [fail-open] docs/normative/authorization/v1/README.md:165 — Missing/unverifiable Fullsend poll invocation provenance is correctly fail-closed, but the positive definition of "trusted Fullsend-controlled invocation provenance" is still unspecified — no binding to a concrete, unforgeable signal is given, and "external callers cannot assert it" states an outcome without a mechanism. This remains the sole authorization principal for starting entity discovery and for state-based matches with no action-indicating element. The "effective platform eligibility policy" fail-closed row still doesn't address whether a permissive wildcard policy is allowed. Unchanged from the prior review round.
  Remediation: Define provenance as bound to a concrete, unforgeable signal the caller cannot set (e.g., an attested workflow/job identity). State that a wildcard/*/all platform eligibility policy requires the same justification bar as other wildcard allowlists in this contract.

Low

• [architecture-coherence] docs/ADRs/0098-entity-first-harness-evaluation.md:115 — ADR 0098 authorizes scheduled entity discovery from "trusted Fullsend-controlled invocation provenance" — i.e., traced to the platform/scheduler, not a human actor. This sits in tension with docs/problems/security-threat-model.md's defense principle that every agent action should trace back to the initiating human, and with docs/architecture.md's Observability section, which still lists attributability as an open question. Unchanged from the prior review round.
  Remediation: Add a short note explaining how attributability is preserved (or is explicitly deferred/out of scope) for poll-triggered runs with no human actor, and consider cross-referencing this ADR from docs/architecture.md's Observability open question.

• [docs-currency] docs/normative/authorization/v1/README.md:6 — Authorization Contract v1 now specifies Fullsend-originated entity discovery (the new section at line 161, its fail-closed rows, and the enforcement diagram's null-event branch) per ADR 0098, but never cites ADR 0098 and carries no forward-looking/not-yet-implemented qualifier — line 6 only references ADR 0054. By contrast, docs/normative/normalized-event/v1/README.md (also touched in this PR) cites both ADR 0054 and ADR 0098 and explicitly labels the entity-first CEL context as "a future entity-first context ... follow-up versioned work." A reader of the authorization contract alone would reasonably conclude the poll-based entity-discovery path is already-shipped policy rather than depending on an accepted-but-unimplemented model. This combines findings raised in the prior review round (missing ADR 0098 citation and missing forward-looking qualifier), both still open.
  Remediation: At line 6 and/or under "Fullsend-originated entity discovery," cite ADR 0098 and add a qualifier mirroring the normalized-event/v1 README's framing (accepted but not yet implemented; the event-backed path above remains the currently shipped mechanism).

• [docs-currency] docs/normative/authorization/v1/README.md:182 — Further trust/injection filtering of retained entity content is still MAY, and docs/architecture.md (poll entity-discovery paragraph, line 329, unchanged since the prior review) still says "trusted input-selection layers authorize retained actor-originated instructions before use," restating the weaker triggering model this same PR revision superseded within this file (triggering of action-indicating elements is now a platform MUST here). This is a cross-document staleness gap rather than a live authorization fail-open, since retained content is explicitly stated to remain untrusted throughout.
  Remediation: Align docs/architecture.md's authorization-gate description with the updated contract: provenance plus the action-indicating-element permission gate authorize dispatch; input-selection layers are defense-in-depth for prompt construction, not the dispatch authorization gate.

---

Note on prior review continuity: This is a re-review (prior head SHA be76891b913ef805900e10ebe859c6b069339fb5). Of the 6 prior findings (2 high, 3 medium, 1 low — plus 1 low from intent-coherence and 1 low from cross-repo-contracts), this round's revisions to docs/normative/authorization/v1/README.md and docs/ADRs/0098-entity-first-harness-evaluation.md substantively addressed the core of both prior high findings (the previously unauthorized-execution and no-input-selection-gate gaps are now covered by a live actor-permission requirement and fail-closed rows), so both are downgraded to medium — real specification gaps remain in scope/definition, not absence of a control. The remaining prior medium and low findings (field-contract fail-open, provenance-definition fail-open, overlay-selection logic error, attributability coherence gap, and the two docs-currency/backward-incompatible framing gaps, the latter two now merged into one finding above) remain open at their prior severities. No new findings from style-conventions.

---

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

High

• [permission-expansion] docs/normative/authorization/v1/README.md:205 — The new "Fullsend-originated entity discovery" path (Enforcement point diagram, line 205) replaces ADR 0054's live actor-role gate with only a trusted-origin check before proceeding straight to CEL evaluation and Execution. Unlike the existing schedule/manual exception, which still runs the standard authorization gate against the service identity, this path never applies the contract's observation (triage) or mutation (write) thresholds to any actor associated with the matched entity. As specified: (1) an unauthorized user's issue/comment becomes eligible for automatic dispatch once a harness with entity sources matches on entity state; (2) content from an actor whose access was later revoked can still drive a dispatched run, since excluded-fields text forbids historical actors as the authorization gate but doesn't require a live re-check before Execution; (3) the "Per-repo configurability" section's claim that a repo "cannot make a stage available to unauthorized users" is undercut, since declaring entity sources on a mutation harness effectively does this via entity content. Linked issue agent: add cron-based trigger for scheduled agent runs #313 does not authorize dropping actor thresholds for mutation-class harnesses.
  Remediation: Specify two platform gates on this path: trusted origin authorizes enumeration and CEL evaluation only; before Execution, require a live forge permission check on a bound actor for actor-originated work, using the same observation vs. mutation thresholds as event-backed dispatch. Deny when no actor can be bound or the live role is below threshold. Update the enforcement diagram and add corresponding fail-closed rows.

• [fail-open] docs/normative/authorization/v1/README.md:170 — The only stated control preventing retained actor-originated entity content from driving the agent is "the harness's trusted input-selection layers," which ADR 0098 explicitly includes harness pre-scripts. The fail-closed table (lines 103-110) has no row for missing, empty, or unverifiable input-selection. An implementation that treats "no pre-script configured" as "proceed without restriction" would be fail-open: all retained comments/bodies become actionable input to the agent. Placing authorization of retained instructions in optional, harness-defined pre-scripts also sits in tension with ADR 0054's platform-level-gate principle.
  Remediation: Add a fail-closed row: missing, empty, or unverifiable input-selection layers ⇒ retained actor-originated content MUST NOT control agent behavior. Make authorization of which retained instructions are actionable a platform-level gate, not solely a harness pre-script.

Medium

• [fail-open] docs/normative/authorization/v1/README.md:167 — The MUST to "omit or minimize unneeded untrusted fields" before CEL evaluation is qualified by "where the normalized entity contract permits" and "retaining ... content ... required by the harness." ADR 0098 defers the field-level entity contract to follow-up normative work, and the fail-closed table has no row covering a missing or incomplete entity contract. Until that contract exists, "where the contract permits" has no defined allowlist boundary, and "required by the harness" lets a harness request injection-bearing fields with no platform ceiling.
  Remediation: Add a fail-closed row: if the versioned normalized-entity contract does not yet enumerate CEL- and prompt-eligible fields, scheduled evaluation is denied (or restricted to a platform-defined allowlist of non-content fields). State that a harness cannot expand the allowlist, and that entity-first Execution should not be enabled before the field-level contract lands.

• [fail-open] docs/normative/authorization/v1/README.md:162 — Missing/unverifiable Fullsend poll invocation provenance is correctly fail-closed, but the positive definition of "trusted Fullsend-controlled invocation provenance" is left unspecified — no binding to a concrete, unforgeable signal is given, and "external callers cannot assert it" states an outcome without a mechanism. Since this is the sole authorization principal for the entire entity-discovery path, an under-specified positive case would collapse the model. The "effective platform eligibility policy" fail-closed row also doesn't address whether a permissive wildcard policy is allowed.
  Remediation: Define provenance as bound to a concrete, unforgeable signal (e.g., an attested workflow/job identity the caller cannot set), and state that a wildcard/*/all platform eligibility policy requires the same justification bar as other wildcard allowlists in this contract.

• [logic-error] docs/ADRs/0098-entity-first-harness-evaluation.md:80 — The Decision states trigger CEL represents a missing event as null, but "overlay when evaluation remains unchanged and uses an empty map guarded with has(event.source)." Overlay selection today distinguishes source-specific configuration via event.source.system (e.g., Jira vs. GitHub overlay blocks). On a fullsend poll scheduled-discovery run for a non-GitHub entity, an overlay when conditioned on event.source.system will not match, silently falling through to a default (e.g., GitHub-scoped) overlay rather than the source-appropriate one. Consequences doesn't address how overlays pick source-specific configuration when there is no event to inspect.
  Remediation: Either extend the overlay when CEL environment to receive the resolved entity so source-specific overlay selection remains possible without an event, or explicitly state in Consequences that overlay selection based on event.source.system will not function on the entity-discovery path until a later specification unifies the two CEL environments.

Low

• [backward-incompatible] docs/normative/authorization/v1/README.md:158 — Unlike docs/normative/normalized-event/v1/README.md (modified in this same PR), which explicitly frames the entity-first CEL context as "a future entity-first context ... follow-up versioned work," the new "Fullsend-originated entity discovery" section presents the trusted-origin gate as an already-live normative exception alongside genuinely-implemented exceptions, with no note that this path depends on ADR 0098's not-yet-implemented model.
  Remediation: Add an explicit forward-looking qualifier to the "Fullsend-originated entity discovery" section (and its fail-closed rows / diagram branch), mirroring the normalized-event/v1 README's framing.

• [architecture-coherence] docs/ADRs/0098-entity-first-harness-evaluation.md — ADR 0098 authorizes scheduled entity discovery without a human actor. This sits in tension with docs/problems/security-threat-model.md's Threat 2 defense consideration that agent actions should be traceable back to the initiating human, and with docs/architecture.md's Observability section, which lists attributability as an open requirement. Neither doc explains how attributability is preserved for poll-triggered runs with no human actor.
  Remediation: Add a short note explaining how attributability is preserved for poll-triggered runs in the absence of a human actor.

The two low findings from the immediately prior review round (wording scope of the platform-eligibility-policy fail-closed row, and the excluded-fields principle statement) have both been resolved in this round — the fail-closed table row now reads "Scheduled evaluations governed by that policy are denied" matching the sibling rows, and the excluded-fields principle now explicitly distinguishes event-backed authorization from Fullsend-originated entity-discovery authorization. No findings from docs-currency or style-conventions review of 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 (4)

Review

Findings

Low

• [spec-ambiguity] docs/normative/authorization/v1/README.md:110 — The fail-closed row for a missing/malformed/unverifiable "effective platform eligibility policy" ("Evaluations governed by that policy are denied") is less explicitly scoped than its two sibling rows ("Entity discovery denied", "That harness is skipped for scheduled evaluation"). The row is contextually scoped to scheduled discovery (it sits directly beneath the two poll-specific rows, and ADR 0098's Scheduling bullet already establishes a deny-by-default posture: "Scheduled entity discovery is denied unless the harness opts in through valid entity sources and effective platform policy permits it"), so this is a wording-consistency nit rather than a live fail-open gap or a risk of the row being misread as governing event-backed dispatch.
  Remediation: Scope the outcome column the same way as the sibling rows, e.g. "Scheduled evaluations governed by that policy are denied." Mirror the same scoped wording in ADR 0098's Scheduling bullet.

• [internal-inconsistency] docs/normative/authorization/v1/README.md:191 — The excluded-fields principle still states that authorization "must be derived from the forge's permission model at event time." The new Fullsend-originated entity-discovery exception (added by this PR, lines 159–173) authorizes from trusted invocation provenance with no event actor and no event time. The exception section and the updated enforcement diagram are specific and accurate, but the governing principle statement was not qualified, so a reader could read provenance-based poll authorization as contradicting the stated principle.
  Remediation: Qualify the principle: event-backed authorization is derived from the forge's permission model at event time; entity-discovery authorization is derived from trusted Fullsend-controlled invocation provenance and must not use historical actors or relationship fields as the dispatch gate.

The finding from the immediately prior review round (the fail-closed table row combining a per-harness condition with a platform-wide condition under one outcome) has been resolved in this round — the row was split into two dedicated rows separating "harness entity sources" from "effective platform eligibility policy," matching the previously requested remediation. No new findings from correctness (beyond the item above), security (beyond the item above), intent-coherence, docs-currency, or style-conventions review of 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 (5)

Review

Findings

Low

• [internal-consistency] docs/normative/authorization/v1/README.md:109 — The Fail-closed-behavior row "Harness entity sources or effective platform eligibility policy are missing or malformed → Scheduled entity discovery denied" still combines a per-harness condition (malformed entity sources) with a platform-wide condition (malformed eligibility policy) under one outcome. ADR 0098's scheduling rule is per-harness ("denied unless the harness opts in ... missing or malformed eligibility data denies evaluation"), so a reader of this table could conclude one harness's malformed entity sources aborts the whole poll cycle rather than just that harness's evaluation. This file is unchanged since the prior review round; this same finding has now been raised across multiple review rounds without being addressed.
  Remediation: Split the row: malformed/unverifiable platform eligibility policy denies the poll run; missing or malformed entity sources deny (skip) only that harness's scheduled evaluation.

• [sub-agent-failure] The challenger sub-agent's adjudication pass on the finding above was discarded per protocol: it returned an empty adjudicated_findings array for a non-empty input set (one finding), which the orchestration protocol treats as an unreliable/failed pass rather than a legitimate full removal. Its own stated removal rationale also cited an incorrect line number (111) for content that is actually at line 109, further supporting discarding its output. Falling back to the pre-challenger finding set (the finding above).

The previously flagged docs/architecture.md:290 stale-doc finding has been resolved in this round — the Dispatch section's ADR 0061 bullet now carries the "partially superseded by ADR 0098" annotation matching the Agent Execution section's treatment. No new findings from correctness, docs-currency, or style-conventions review of the current diff.

Previous run (6)

Review

Findings

Low

• [internal-consistency] docs/normative/authorization/v1/README.md:109 — The Fail-closed-behavior row "Harness entity sources or effective platform eligibility policy are missing or malformed → Scheduled entity discovery denied" still combines a per-harness condition (malformed entity sources) with a platform-wide condition (malformed eligibility policy) under one outcome. ADR 0098's scheduling rule is per-harness ("denied unless the harness opts in ... missing or malformed eligibility data denies evaluation"), so a reader of this table could conclude one harness's malformed entity sources aborts the whole poll cycle rather than just that harness's evaluation. This file is unchanged since the prior review round.
  Remediation: Split the row: malformed/unverifiable platform eligibility policy denies the poll run; missing or malformed entity sources deny (skip) only that harness's scheduled evaluation.

• [stale-doc] docs/architecture.md:290 — In the Dispatch "Decided" list, the ADR 0061 bullet still describes routing as operating on a NormalizedEvent with no supersession note, while the Agent Execution section (lines 157-158) annotates ADR 0061 as "partially superseded by ADR 0098." The following ADR 0098 bullet (lines 302-303) only names ADR 0063 as partially superseded. The two historical citations (ADR 0061 at line 290 and ADR 0063 at line 294) are treated identically as unannotated, so this is a minor gap rather than a contradiction, but the Dispatch section's supersession notes are inconsistent with Agent Execution's.
  Remediation: On the Dispatch ADR 0098 bullet, also note partial supersession of ADR 0061 (matching Agent Execution's treatment), and/or add a short "partially superseded by ADR 0098" marker on the Dispatch section's ADR 0061 citation.

---

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

Low

• [internal-consistency] docs/ADRs/0098-entity-first-harness-evaluation.md:119 — The Consequences bullet says "harness authors maintain one predicate across event and polling contexts; only harnesses opting into entity sources must guard event access with event != null." But per Candidate sources (event set to null on the poll path) and the Predicate context MUST, a null-guarded wrapper of an existing event-only rule (event != null && event.transition.kind == "comment_added" && ...) is always false when polling — the harness would enumerate entities and never match. That undercuts the ADR's own stated goal (recovering durable state such as an unhandled /fs-fix comment) unless the predicate also carries a clause that can succeed when event is null.
  Remediation: Qualify the Consequences bullet (and the Predicate context MUST) so the null check is described as a crash-safety rule only, and state that entity-source harnesses intended to match on scheduled discovery must also express a condition that can succeed when event is null (i.e., based on entity/activity state).

• [internal-consistency] docs/normative/authorization/v1/README.md:109 — The new Fail-closed-behavior row "Harness entity sources or effective platform eligibility policy are missing or malformed → Scheduled entity discovery denied" combines a per-harness condition (malformed entity sources) with a platform-wide condition (malformed eligibility policy) under one outcome. ADR 0098's scheduling rule is per-harness ("denied unless the harness opts in ... missing or malformed eligibility data denies evaluation"), so a reader of this table could conclude one harness's malformed entity sources aborts the whole poll cycle rather than just that harness's evaluation.
  Remediation: Split the row: malformed/unverifiable platform eligibility policy denies the poll run; missing or malformed entity sources deny (skip) only that harness's scheduled evaluation.

---

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

Low

• [fail-open] docs/ADRs/0098-entity-first-harness-evaluation.md:98 — This revision closes most of the prior auth-gap finding: the Authorization bullet (lines 103–110) now states scheduled fullsend poll entity discovery is authorized only by trusted Fullsend-controlled invocation provenance and denies callers that cannot establish it, ADR 0054 carries a reciprocal annotation, and docs/normative/authorization/v1/README.md's new "Fullsend-originated entity discovery" section and rewritten "Enforcement point" diagram add a matching gate. One narrower gap remains: the Scheduling bullet (lines 98–102) says harness enablement and platform policy "determine whether scheduled evaluation is permitted" without stating the default when that enablement/policy configuration is absent, empty, or malformed, and the Authorization Contract v1 "Fail-closed behavior" table (still actor-role-only) was not extended with a row for this new gate. This is a documentation-completeness gap, not a live fail-open bug — the PR ships no code.
  Remediation: State in the Scheduling bullet that scheduled entity discovery is denied unless harness enablement and platform policy are both explicitly configured (absence/empty/malformed configuration denies), or explicitly note that this default is deferred to the same versioned spec already called out for the field-level contract. Optionally add a row to the Authorization Contract v1 Fail-closed behavior table for the provenance gate.

• [incomplete-doc] docs/normative/normalized-event/v1/README.md:12 — The Contract section still states harness trigger CEL receives a single root variable event bound to a NormalizedEvent, and that authorization is a single post-normalization gate before CEL. This file is not touched by this PR and remains accurate for the currently shipped event-backed dispatch path. However, this round widens the gap between adjacent v1 normative documents: docs/normative/authorization/v1/README.md (updated here) now diagrams a second, parallel poll path where CEL evaluates with event null and is gated by trusted origin rather than an event actor, while ADR 0061 is annotated as superseded only for its event-only CEL context. The sibling specs are drifting apart without a cross-reference.
  Remediation: Add a short note to the Contract's CEL-context (and optionally Authorization) bullets stating that ADR 0098 adopts entity-first evaluation (required entity + nullable event) and that this NormalizedEvent v1 contract continues to describe the shipped event-backed system until the versioned spec/implementation lands.

• [internal-consistency] docs/normative/authorization/v1/README.md:215 — The rewritten "Enforcement point" section (above) now defines a poll path whose CEL evaluation has event null, gated by trusted invocation provenance rather than an event actor. The following "CEL triggers: routing only" subsection was not updated to match: it still states unqualified that "IsAuthorized() runs before MatchHarnesses() in the dispatch core" and that "an event that fails authorization never reaches CEL evaluation" — true only for the event-backed path, since IsAuthorized()/Dispatch() require a non-nil NormalizedEvent today. Read literally, it can be misread as requiring poll to reconstruct an event, contradicting the diagram directly above it and ADR 0098.
  Remediation: Qualify the sentence so IsAuthorized() before MatchHarnesses() is identified as the event-backed mechanism, and add that the poll path's equivalent is the trusted-origin gate before entity enumeration/CEL (event remains null).

---

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

• [auth-gap] docs/ADRs/0098-entity-first-harness-evaluation.md:96 — The Scheduling/Authorization decisions introduce scheduled entity discovery with no scheduling principal, permitted by "harness enablement and platform policy," while claiming ADR 0054 still authorizes event-triggered dispatch and that "the centralized authorization boundary" is not weakened. The living Authorization Contract v1 states the sole enforcement point is inside fullsend dispatch after NormalizedEvent normalization and before CEL — IsAuthorized(event *normevent.Event) bool structurally requires an event (IsAuthorized(nil) denies). This PR ships no code, and the ADR does not say "fail open," but it also doesn't pin a fail-closed default for missing/empty/malformed enablement or platform policy, doesn't say how the new path relates to IsAuthorized/the enforcement-point pipeline (new gate vs. a scoped exception), and — unlike its own annotations of ADR 0061 and ADR 0063 — doesn't annotate ADR 0054 or the Authorization Contract v1 it's bound to. docs/architecture.md:324 restates the principal-less path as a settled "Decided" fact, overstating the maturity of an unresolved contract relationship.
  Remediation: In ADR 0098's Authorization decision, state a fail-closed default (scheduled entity discovery denied unless harness enablement and platform policy are both explicitly configured; absence/empty/malformed configuration denies) and specify how this interacts with IsAuthorized / Authorization Contract v1 (new code path, or an explicit, scoped exception documented in that living contract). Add the same Status-style supersession annotation used for ADR 0061/0063 onto ADR 0054, add a pointer in Authorization Contract v1, and mirror the fail-closed wording in the docs/architecture.md authorization-gate bullet.

Low

• [design-alignment] docs/ADRs/0098-entity-first-harness-evaluation.md:70 — The trigger contract pins a missing prompting event as CEL null and requires event != null guards. That's internally consistent for the harness trigger predicate, but it's a different sentinel than the existing overlay when convention (ResolveOverlays substitutes an empty map for a nil event; docs instruct has(event.source)). Trigger and overlay are separate CEL environments today, so the ADR isn't wrong, but it never states whether scheduled discovery will keep these sentinels distinct or eventually share one event value across trigger and overlay evaluation.
  Remediation: Add one sentence to the Predicate context bullet: trigger CEL uses CEL null (event != null); overlay when evaluation is unchanged (empty map + has(event.source)), unless a later spec intentionally unifies them.

• [scope-alignment] docs/architecture.md:314 — The rewritten Dispatch authorization-gate bullet stacks two different scheduled-dispatch models in one item: existing schedule/manual dispatch represented as a NormalizedEvent whose actor is the configured service identity, then (line 324) "Scheduled entity discovery has no scheduling principal." The mechanisms are named differently, so this isn't a strict contradiction, but a reader can still read "scheduled dispatch" as sometimes having a principal and sometimes not, with no explicit rule for which mechanism applies when.
  Remediation: Split into two labeled items, or add a clause naming the mechanisms explicitly (cron/manual paths that synthesize a NormalizedEvent vs. ADR 0098's principal-less entity discovery / fullsend poll).

• [stale-reference] docs/normative/normalized-event/v1/README.md:12 — The Contract section still says harness trigger CEL receives a single root variable event. ADR 0098 (Accepted) changes that to a required entity plus a nullable event. ADR 0098 defers "the field-level contract and query-planning protocol" to a versioned spec, which reads as the entity-history/query-planning work rather than an explicit deferral of this CEL-root-variable sentence — so the living normative contract and the new ADR are related only implicitly.
  Remediation: Add a short note on the Contract's CEL-context bullet (or in ADR 0098's Consequences) stating that the trigger CEL root-variable update is deferred until the versioned spec/implementation lands, so v1 continues to accurately describe the shipped system in the interim.

• [scope-creep] docs/architecture.md:328 — The Dispatch "Open questions" list drops "Is GitHub's event system sufficient for forge-native duplicate protection... (Jira polling per ADR 0063 uses entity-property locks and runner lock refresh.)" and replaces it with an unrelated ADR 0098 entity-history/query-planning question in the same slot. ADR 0098 does not resolve GitHub duplicate-dispatch/idempotency; ADR 0063's coordination decisions remain current. Silent replacement doesn't track resolution of the removed question.
  Remediation: Restore the duplicate-protection question, or fold it into the remaining coordinator-state open question with an explicit note that ADR 0098 did not resolve it.

---

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

Findings

Low

• [internal-consistency] docs/architecture.md:290 — The Dispatch "Decided" section contains an ADR 0061 bullet at lines 287–290 that describes routing as "operating on a NormalizedEvent struct" without a supersession annotation. The Harness section at lines 151–158 already documents the partial supersession by ADR 0098 (with the updated entity+event context), but the Dispatch section’s ADR 0061 bullet gives a conflicting account of the CEL input. The ADR 0098 bullet at lines 302–303 correctly scopes its supersession annotation to ADR 0063 (event-reconstruction).
  Remediation: Add a short parenthetical to the ADR 0061 bullet at line 290 noting that the evaluation context is partially superseded by ADR 0098.

• [scope-alignment] docs/ADRs/0098-entity-first-harness-evaluation.md — Issue agent: add cron-based trigger for scheduled agent runs #313 requests cron-based trigger support; this ADR captures the prerequisite entity-first evaluation architecture that enables scheduled dispatch. The PR body states "This captures the architecture discussion in agent: add cron-based trigger for scheduled agent runs #313" rather than claiming to close it. Framing is correct and the issue remains open.

• [scope-alignment] docs/ADRs/0098-entity-first-harness-evaluation.md:121 — The consequences section acknowledges "ADR 0063’s event-reconstruction path still requires a migration plan for polling drivers" but provides no issue reference or follow-up tracking anchor.
  Remediation: File a follow-up issue for the polling-driver migration plan and add its reference to the consequences bullet.

• [stale-normative-spec] docs/normative/normalized-event/v1/README.md:12 — The Contract section describes a single event root variable for harness CEL triggers. ADR 0098 changes the context to a required entity and a nullable event. ADR 0098 explicitly defers the normative contract update, so this is still accurate for the working system.

• [stale-user-guide] docs/guides/user/cel-triggers-reference.md:40 — The "Writing CEL triggers" section states the trigger expression has a single root variable event. Under ADR 0098, harnesses with entity sources receive a required entity and a nullable event. The implementation has not yet landed, so the guide accurately describes the working system today.

• [stale-reference-comment] docs/reference/harness-reference.md:14 — The inline YAML comment on the trigger field reads "Optional CEL expression over NormalizedEvent." Under ADR 0098, the context becomes entity + optional event, but the comment still accurately describes the working system.

• [stale-contributing-doc] docs/contributing/harness-fields.md:63 — The trigger field rationale reads "evaluated against normalized events (ADR-0061)." ADR 0098 partially supersedes ADR 0061’s event-only context, but the field classification remains correct and the implementation has not changed.

• [stale-user-guide] docs/guides/user/bring-your-own-agent.md:90 — The dispatch flow describes only the event-driven dispatch path. ADR 0098 adds scheduled entity discovery as a parallel dispatch path, but the implementation has not landed.

• [stale-adr-annotation] docs/ADRs/0067-gitlab-cron-polling-event-dispatch.md:25 — ADR 0067 describes GitLab cron-polling that emits a NormalizedEvent per detected change — the concrete implementation of ADR 0063’s event-reconstruction requirement. The PR annotates ADR 0063 to note that event-reconstruction is superseded by ADR 0098, but ADR 0067 has no corresponding supersession annotation.
  Remediation: Add a short annotation to ADR 0067’s Status section noting that ADR 0098 supersedes the requirement for poll drivers to reconstruct changes as NormalizedEvents.

---

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

Review

Findings

Medium

• [internal-inconsistency] docs/architecture.md:151 — The Agent Harness "Decided" section at line 151–154 still describes harness trigger expressions as "evaluated against a forge-neutral NormalizedEvent" and references only ADR 0061. The PR updated the Dispatch section (around line 292) to describe entity-first evaluation per ADR 0106, but left the Harness section bullet unchanged. The two sections of the same document now give contradictory accounts of the harness trigger input contract.
  Remediation: Update the Harness trigger expressions bullet to describe the entity + optional-event CEL context and add a reference to ADR 0106 alongside the existing ADR 0061 link.

Low

• [internal-consistency] docs/architecture.md:283 — Within the Dispatch "Decided" section, the ADR 0061 bullet at lines 283–286 describes routing "operating on a NormalizedEvent struct" in present tense. The new ADR 0106 bullet at lines 292–299 describes "one CEL predicate over a required normalized entity and an optional prompting event," partially superseding the ADR 0061 evaluation context. The ADR 0106 bullet's supersession annotation at lines 298–299 mentions only ADR 0063, omitting ADR 0061 despite ADR 0106's own Status section explicitly stating it partially supersedes both ADRs.
  Remediation: Annotate the ADR 0061 routing bullet at line 286 with a note such as "(evaluation context superseded by ADR 0106)" and add "and ADR 0061" to the supersession annotation at lines 298–299.

• [scope-alignment] docs/ADRs/0106-entity-first-harness-evaluation.md — Issue agent: add cron-based trigger for scheduled agent runs #313 requests cron-based trigger support; this ADR addresses entity-first evaluation as its prerequisite. The PR correctly frames agent: add cron-based trigger for scheduled agent runs #313 as context rather than claiming to close it, and the issue remains open.

• [stale-normative-spec] docs/normative/normalized-event/v1/README.md:12 — The Contract section describes a single event root variable for harness CEL triggers. ADR 0106 changes the context to a required entity and a nullable event. ADR 0106 explicitly defers the normative contract update to a future versioned specification.

• [stale-user-guide] docs/guides/user/cel-triggers-reference.md:40 — The "Writing CEL triggers" section states the trigger expression has a single root variable event. Under ADR 0106, the context will supply entity and nullable event. The implementation has not yet landed, so the guide is still accurate for the working system.

• [stale-reference-comment] docs/reference/harness-reference.md:14 — The inline YAML comment on the trigger field reads "Optional CEL expression over NormalizedEvent." Under entity-first evaluation the context becomes entity + optional event, but the comment still accurately describes the working system.

• [stale-contributing-doc] docs/contributing/harness-fields.md:63 — The trigger field classification reads "evaluated against normalized events (ADR-0061)." ADR 0106 partially supersedes ADR 0061's event-only context, but the core field classification remains correct and the implementation has not changed.

• [stale-user-guide] docs/guides/user/bring-your-own-agent.md:38 — The dispatch flow describes only the event-driven path. ADR 0106 adds scheduled entity discovery as a parallel dispatch path, but the implementation has not landed.

---

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

• [internal-inconsistency] docs/architecture.md:151 — The Agent Harness "Decided" section still describes harness trigger expressions as "evaluated against a forge-neutral NormalizedEvent" and references only ADR 0061. The Dispatch section (updated by this PR) now describes the evaluation context as "a required normalized entity and an optional prompting event" per ADR 0106. The same document contains contradictory descriptions of the trigger evaluation context.
  Remediation: Update the Harness section bullet (lines 151–154) to reference ADR 0106's entity-first context, or add a cross-reference noting the evaluation context evolution.

Low

• [pattern-inconsistency] docs/ADRs/0106-entity-first-harness-evaluation.md:19 — ADR 0106's Status section lists only ADR 0063 as partially superseded, but the amendment added to ADR 0061 (by this same PR) explicitly states that ADR 0061's event-only CEL context is superseded by ADR 0106. The established convention is for the superseding ADR to enumerate all ADRs it supersedes.
  Remediation: Add a supersession sentence for ADR 0061 to the Status section of ADR 0106, parallel to the existing ADR 0063 sentence.

• [internal-inconsistency] docs/architecture.md:283 — Within the Dispatch "Decided" section, the ADR 0061 bullet (lines 283–286) describes routing via CEL triggers "operating on a NormalizedEvent struct," while the new ADR 0106 bullet (lines 292–299) describes "one CEL predicate over a required normalized entity and an optional prompting event." These are sequential decisions in a chronological list, but the adjacent terminology could confuse readers.
  Remediation: Optionally annotate the ADR 0061 routing bullet to note the evaluation context evolution.

• [stale-reference] docs/architecture.md:301 — The Conversation participation bullet describes conversations entering dispatch "as NormalizedEvent entities with entity.kind: conversation." Under the entity-first model, conversations would enter as resolved entities evaluated by harness predicates.
  Remediation: Update to align with entity-first framing.

• [stale-reference] docs/architecture.md:309 — The Dispatch authorization gate characterizes "all agent dispatch paths" as "slash commands and automatic event triggers." ADR 0106 introduces scheduled entity discovery as a new dispatch path not listed here.
  Remediation: Add scheduled entity discovery to the parenthetical list of dispatch paths.

• [architectural-coherence] docs/architecture.md:171 — The removed open question about forge-native duplicate protection was replaced rather than explicitly resolved. The entity-first model's per-harness processing receipts subsume the concern, but the resolution path is implicit.
  Remediation: Consider a brief note clarifying that per-harness processing receipts serve as the durable deduplication boundary.

• [stale-normative-spec] docs/normative/normalized-event/v1/README.md:12 — The Contract section describes a single event root variable for harness CEL triggers. ADR 0106 changes the context to include a required entity and a nullable event. However, ADR 0106 explicitly defers the normative contract ("The field-level contract and query planning protocol belong in a versioned normative specification"); updating the v1 spec before implementation would describe behavior that does not yet exist.

• [stale-user-guide] docs/guides/user/cel-triggers-reference.md:40 — The "Writing CEL triggers" section describes a single event root variable. ADR 0106 changes the model, but the implementation has not landed — the guide accurately describes the current CEL context.

• [stale-reference-comment] docs/reference/harness-reference.md:14 — The inline YAML comment on the trigger field reads "Optional CEL expression over NormalizedEvent." Under entity-first evaluation this becomes entity + optional event, but the current comment accurately describes the working system.

• [stale-contributing-doc] docs/contributing/harness-fields.md:63 — The trigger field classification says "evaluated against normalized events (ADR-0061)." ADR 0106 partially supersedes this, but the key classification ("not forge-specific") remains correct.

• [stale-user-guide] docs/guides/user/bring-your-own-agent.md:38 — Describes only the event-driven dispatch path. ADR 0106 adds scheduled entity discovery but the implementation has not landed; the guide accurately describes current behavior.

• [scope-alignment] docs/ADRs/0106-entity-first-harness-evaluation.md — Issue agent: add cron-based trigger for scheduled agent runs #313 requests cron-based trigger support. This ADR addresses a related but distinct concern: entity-first evaluation as the prerequisite for cron scheduling. The PR correctly frames agent: add cron-based trigger for scheduled agent runs #313 as context rather than claiming to close it.

---

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

Critical

• [numbering-collision] docs/ADRs/0099-entity-first-harness-evaluation.md:1 — ADR number 0099 is already taken on main by 0099-codex-agent-runtime.md ("99. Codex credential delivery — custom model provider with a runner-seeded token file", accepted 2026-09-02). Multiple existing files reference ADR 0099 as the codex runtime decision (docs/architecture.md lines 227, 238, 248; docs/ADRs/0100-codex-sandbox-hooks.md). This PR creates a second file with the same number prefix, and the resulting architecture.md would contain two different [ADR 0099] links pointing to different files.
  Remediation: Renumber this ADR to the next available number (0101+, since 0100 exists). Rename the file, update the frontmatter title, heading, and all references in the changed files (0061, 0063, architecture.md, agent-architecture.md).

Medium

• [internal-inconsistency] docs/architecture.md:146 — The Agent Harness section (line 146–149) states harness trigger expressions are "evaluated against a forge-neutral NormalizedEvent" and only references ADR 0061. The Dispatch section (modified by this PR) now says the predicate context is "a required normalized entity and an optional prompting event" per ADR 0099. After this PR lands, the same living architecture document will contain two contradictory descriptions of the harness CEL evaluation context.
  Remediation: Update the Harness section bullet (line 146–149) to reflect the entity-first evaluation context, or add a forward reference noting that the evaluation context is governed by the entity-first decision described in the Dispatch section.

Low

• [stale-reference] docs/architecture.md:287 — The Conversation participation bullet (outside the diff) still describes conversations entering dispatch "as NormalizedEvent entities" with the old event-only model language. Should be reviewed for alignment with entity-first framing.
  Remediation: Review and update the Conversation participation bullet to align with entity-first framing.

• [scope-alignment] docs/ADRs/0099-entity-first-harness-evaluation.md — The referenced issue agent: add cron-based trigger for scheduled agent runs #313 requests cron-based trigger support. This ADR addresses a related but distinct concern: replacing event-only CEL evaluation with an entity-first model. The PR correctly frames agent: add cron-based trigger for scheduled agent runs #313 as context rather than claiming to close it.

• [edge-case] docs/ADRs/0099-entity-first-harness-evaluation.md:96 — The Consequences section states harness authors "must guard event access when event is null" but the Decision section does not specify the CEL null-guard mechanism or what happens when a predicate accesses event fields without guarding.
  Remediation: Consider noting whether the platform should provide a null-safe accessor (e.g., has(event)) or whether authors use CEL's native has() macro.

• [cross-reference-phrasing] docs/ADRs/0061-harness-cel-dispatch.md:24 — The annotation "The event-only CEL context is superseded by [ADR 0099]..." is placed directly under the Status header without explicitly stating the rest of ADR 0061 remains valid. Could mislead readers into thinking 0061 is fully superseded.
  Remediation: Clarify that only the event-only CEL context aspect is superseded, not the entire ADR.

---

Labels: PR introduces an ADR about dispatch and harness evaluation, modifies architecture and problem docs

---

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]

🤖 Review · ❌ Terminated · Started 6:07 AM UTC · Ended 6:31 AM UTC

Commit: 3bf2eff · View workflow run →

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:07 AM UTC · Completed 6:31 AM UTC

Commit: 3bf2eff · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.93

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:29 AM UTC · Completed 11:45 AM UTC

Commit: ee8ecfe · View workflow run →

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

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:51 AM UTC · Completed 12:10 PM UTC

Commit: b1b82cf · View workflow run →

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

Superseded by updated review