Skip to content

docs(adr): adopt entity-first harness evaluation - #6956

Open
ifireball wants to merge 4 commits into
fullsend-ai:mainfrom
ifireball:codex/entity-first-harness-evaluation
Open

docs(adr): adopt entity-first harness evaluation#6956
ifireball wants to merge 4 commits into
fullsend-ai:mainfrom
ifireball:codex/entity-first-harness-evaluation

Conversation

@ifireball

@ifireball ifireball commented Sep 3, 2026

Copy link
Copy Markdown
Member

Summary

  • add ADR 0106 adopting one harness CEL predicate over a resolved entity and optional prompting event
  • 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's configured identity
  • partially supersede ADR 0063's 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 0106
  • 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 keep one predicate that may inspect either or both contexts.

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's configured agent identity and permissions.

The detailed normalized-entity and query-planning contract remains follow-up normative work. Singleton scheduled runs without a natural entity remain outside this ADR.

This captures the architecture discussion in #313. Stable entity identity can support the separate serialized follow-up model accepted in ADR 0098.

Verification

  • pre-commit run --files docs/ADRs/0061-harness-cel-dispatch.md docs/ADRs/0063-polling-based-work-discovery.md docs/ADRs/0106-entity-first-harness-evaluation.md docs/architecture.md docs/problems/agent-architecture.md
  • make lint

Use one CEL predicate over a resolved entity and optional prompting event. Supersede event-reconstruction polling while retaining compatible poll driver machinery.

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

Copy link
Copy Markdown

PR Summary by Qodo

Adopt entity-first harness evaluation architecture

📝 Documentation 🕐 20-40 Minutes

Grey Divider

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

fullsend-ai-review Bot commented Sep 3, 2026

Copy link
Copy Markdown

🤖 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

qodo-code-review Bot commented Sep 3, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

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



Remediation recommended

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 📘 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



Informational

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


Grey Divider

Context sources
✅ Compliance rules (platform): 65 rules
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread docs/ADRs/0099-entity-first-harness-evaluation.md Outdated
Comment thread docs/ADRs/0061-harness-cel-dispatch.md Outdated
Comment thread docs/architecture.md
Comment thread docs/ADRs/0106-entity-first-harness-evaluation.md
Comment thread docs/ADRs/0063-polling-based-work-discovery.md Outdated
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

Site preview

Preview: https://785b73d6-site.fullsend-ai.workers.dev

Commit: 016d5bd7f5e3078cab289f34764a57c6ccf13845

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

fullsend-ai-review Bot commented Sep 3, 2026

Copy link
Copy Markdown

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

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

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

fullsend-ai-review Bot commented Sep 3, 2026

Copy link
Copy Markdown

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

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

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[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

Renumber the ADR after the collision on main, clarify partial supersession and authorization, and preserve the existing architecture narrative.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
Integrate the latest main before updating the pull request.

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

fullsend-ai-review Bot commented Sep 6, 2026

Copy link
Copy Markdown

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

Commit: 3bf2eff · View workflow run →

@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 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

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

fullsend-ai-review Bot commented Sep 6, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:35 AM UTC · Completed 6:55 AM UTC

Commit: 016d5bd · View workflow run →

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

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

  • docs/architecture.md (file-level): Line 151 · [medium] internal-inconsistency

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.

Suggested fix: 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.

  • docs/architecture.md (file-level): Line 283 · [low] internal-consistency

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.

Suggested fix: 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.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/dispatch Workflow dispatch and triggers component/docs User-facing documentation risk/low PR risk: low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant