feat(session): add session store event-sourcing protobuf contracts#501
Open
yordis wants to merge 15 commits into
Open
feat(session): add session store event-sourcing protobuf contracts#501yordis wants to merge 15 commits into
yordis wants to merge 15 commits into
Conversation
yordis
commented
Jul 24, 2026
yordis
commented
Member
- ADR#0035 decided the session store as a decider aggregate on NATS JetStream but deferred the concrete wire contracts to implementation follow-up; this supplies that event-sourcing schema so the store can be built on the existing decider substrate rather than the prior art's hand-rolled JetStream/KV persistence.
Realizes the event schema decided in ADR#0035 so the session store can be built on the decider substrate. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
PR SummaryMedium Risk Overview ADR#0035 is expanded to match that schema: proto path Adds Reviewed by Cursor Bugbot for commit 579ca5c. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds the v1alpha1 session-event protobuf model, Rust codec and validation support, ADR and glossary documentation, feature-aware decoding tests, and CI checks for proto compatibility and generated bindings. ChangesSession Event Contracts
Proto CI Automation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@proto/trogonai/session/sessions/v1/artifact.proto`:
- Around line 12-18: Replace LEGACY_REQUIRED with EXPLICIT for required contract
fields in proto/trogonai/session/sessions/v1/artifact.proto lines 12-18,
proto/trogonai/session/sessions/v1/digest.proto lines 11-13,
proto/trogonai/session/sessions/v1/tool_call.proto line 19, and
proto/trogonai/session/sessions/v1/message.proto lines 16-18. Add boundary
validation for these fields, preserving an exception only where ADR#0035
explicitly requires historical-event compatibility.
In `@proto/trogonai/session/sessions/v1/cascade_policy.proto`:
- Around line 8-15: Reject CASCADE_POLICY_UNSPECIFIED at the command/decider
boundary for both SubagentLinked and DelegationDispatched before either link
fact is written. Validate that cascade_policy is explicitly
CASCADE_POLICY_CASCADE_ON_PARENT_TERMINAL or CASCADE_POLICY_INDEPENDENT, while
preserving the existing missing-field presence validation.
In `@proto/trogonai/session/sessions/v1/file_changed.proto`:
- Around line 12-13: Update the file-change persistence writer/runtime
validation around FileChangeKind and previous_path to reject
FILE_CHANGE_KIND_UNSPECIFIED and require a non-empty previous_path whenever the
kind is RENAMED. Preserve valid non-rename events, and enforce these invariants
before persistence rather than relying on LEGACY_REQUIRED.
In `@proto/trogonai/session/sessions/v1/tool_call.proto`:
- Around line 10-13: The typed oneof values used for persisted message events
currently allow empty Default/Some constructions. Update the builders for
ToolCallResult and ContentBlock::...::new(...) to validate that their oneof kind
is populated with a valid non-empty value before appending or persisting; reject
empty ToolCallResult::default() and ContentBlock variants containing default
image references. Apply this across tool_call.proto lines 10-13 and
message.proto lines 31-37, preserving valid text, artifact, and other content
variants.
In `@proto/trogonai/session/sessions/v1/user_message_recorded.proto`:
- Around line 10-11: The concrete persisted event fields currently use
LEGACY_REQUIRED, which can reject historical events missing fields. In
user_message_recorded.proto lines 10-11, assistant_message_started.proto lines
10-12, assistant_message_completed.proto lines 12-13, tool_call_requested.proto
lines 8-12, tool_call_started.proto lines 8-10, tool_call_approved.proto lines
8-11, tool_call_completed.proto lines 10-13, tool_call_failed.proto lines 8-11,
artifact_recorded.proto lines 10-11, and file_changed.proto lines 10-12, replace
LEGACY_REQUIRED with explicit presence semantics; enforce any new requiredness
at the aggregate boundary instead of these event message definitions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a8290457-2aa2-4638-86be-22d6d406a0b1
⛔ Files ignored due to path filters (78)
rsworkspace/crates/platform/trogonai-proto/src/gen/mod.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.artifact.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.artifact.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.artifact_recorded.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.artifact_recorded.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_completed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_completed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_started.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_started.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.cascade_policy.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.compacted.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.compacted.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.delegation_dispatched.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.delegation_dispatched.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.digest.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.digest.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_attempt_ended.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_attempt_ended.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_attempt_ready.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_attempt_ready.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_attempt_started.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_attempt_started.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_plan.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_plan.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.file_change.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.file_changed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.file_changed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.mod.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.operation_outcome_recorded.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.operation_outcome_recorded.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.operation_reserved.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.operation_reserved.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_cancelled.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_cancelled.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_closed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_closed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_deleted.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_deleted.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_failed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_failed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_forked.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_forked.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_rewound.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_rewound.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_started.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_started.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.subagent_detached.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.subagent_detached.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.subagent_linked.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.subagent_linked.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.subagent_parent_terminated.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.subagent_parent_terminated.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.token_usage.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.token_usage.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_approved.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_approved.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_completed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_completed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_failed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_failed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_requested.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_requested.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_started.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_started.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.user_message_recorded.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.user_message_recorded.rsis excluded by!**/gen/**
📒 Files selected for processing (36)
proto/trogonai/session/sessions/v1/artifact.protoproto/trogonai/session/sessions/v1/artifact_recorded.protoproto/trogonai/session/sessions/v1/assistant_message_completed.protoproto/trogonai/session/sessions/v1/assistant_message_started.protoproto/trogonai/session/sessions/v1/cascade_policy.protoproto/trogonai/session/sessions/v1/compacted.protoproto/trogonai/session/sessions/v1/delegation_dispatched.protoproto/trogonai/session/sessions/v1/digest.protoproto/trogonai/session/sessions/v1/events.protoproto/trogonai/session/sessions/v1/execution_attempt_ended.protoproto/trogonai/session/sessions/v1/execution_attempt_ready.protoproto/trogonai/session/sessions/v1/execution_attempt_started.protoproto/trogonai/session/sessions/v1/execution_plan.protoproto/trogonai/session/sessions/v1/file_change.protoproto/trogonai/session/sessions/v1/file_changed.protoproto/trogonai/session/sessions/v1/message.protoproto/trogonai/session/sessions/v1/operation_outcome_recorded.protoproto/trogonai/session/sessions/v1/operation_reserved.protoproto/trogonai/session/sessions/v1/session_cancelled.protoproto/trogonai/session/sessions/v1/session_closed.protoproto/trogonai/session/sessions/v1/session_deleted.protoproto/trogonai/session/sessions/v1/session_failed.protoproto/trogonai/session/sessions/v1/session_forked.protoproto/trogonai/session/sessions/v1/session_rewound.protoproto/trogonai/session/sessions/v1/session_started.protoproto/trogonai/session/sessions/v1/subagent_detached.protoproto/trogonai/session/sessions/v1/subagent_linked.protoproto/trogonai/session/sessions/v1/subagent_parent_terminated.protoproto/trogonai/session/sessions/v1/token_usage.protoproto/trogonai/session/sessions/v1/tool_call.protoproto/trogonai/session/sessions/v1/tool_call_approved.protoproto/trogonai/session/sessions/v1/tool_call_completed.protoproto/trogonai/session/sessions/v1/tool_call_failed.protoproto/trogonai/session/sessions/v1/tool_call_requested.protoproto/trogonai/session/sessions/v1/tool_call_started.protoproto/trogonai/session/sessions/v1/user_message_recorded.proto
|
Code Coverage SummaryDetailsDiff against mainResults for commit: 579ca5c Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Audited the event set against the nine session-store research products; these facts (denied tool calls, failed/finished assistant turns, produced checkpoints, opaque reasoning, priced usage) were persisted there but missing here, and each must be a recorded fact so deterministic audit/cost/outcome projections rebuild from the log alone. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
A field-level audit of the event payloads against the nine research products and the platform-branch prior art surfaced recorded facts we were flattening or dropping; each added field must be on the log so deterministic projections (model-visible context, cost, file diffs, checkpoint restore, compaction effect) rebuild from it alone. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
proto/trogonai/session/sessions/v1/artifact.proto (2)
30-38: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftUse explicit presence across the new event contracts.
LEGACY_REQUIREDmakes omitted fields wire-invalid; useEXPLICITand perform semantic validation before persistence, unless ADR#0035 explicitly requires wire-required compatibility. (protobuf.dev)
proto/trogonai/session/sessions/v1/artifact.proto#L30-L38: change storage metadata fields to explicit presence and validate them conditionally.proto/trogonai/session/sessions/v1/artifact.proto#L44-L48: change required availability metadata to explicit presence and validate it before persistence.proto/trogonai/session/sessions/v1/token_usage.proto#L21-L23: validate amount and currency at the cost writer boundary.proto/trogonai/session/sessions/v1/message.proto#L26-L26: validate message timestamps without wire-required parsing.proto/trogonai/session/sessions/v1/message.proto#L55-L55: validate thinking content at event construction.proto/trogonai/session/sessions/v1/checkpoint.proto#L13-L19: validate complete checkpoint identity and digest before persistence.proto/trogonai/session/sessions/v1/execution_attempt_started.proto#L13-L16: validate session, attempt, plan digest, and attempt number before persistence.proto/trogonai/session/sessions/v1/execution_attempt_started.proto#L20-L21: validate host reference and digest before persistence.proto/trogonai/session/sessions/v1/checkpoint_produced.proto#L13-L15: validate the produced checkpoint fact before persistence.proto/trogonai/session/sessions/v1/checkpoint_produced.proto#L19-L19: validate the checkpoint sequence at the writer boundary.proto/trogonai/session/sessions/v1/session_forked.proto#L12-L14: validate fork identifiers and history base sequence before persistence.proto/trogonai/session/sessions/v1/session_forked.proto#L17-L17: validate the concrete fork reason before persistence.proto/trogonai/session/sessions/v1/compacted.proto#L16-L20: validate compaction identity, summary, and boundaries before persistence.proto/trogonai/session/sessions/v1/compacted.proto#L22-L22: validate the concrete compaction trigger before persistence.proto/trogonai/session/sessions/v1/assistant_message_completed.proto#L13-L15: validate completion facts before persistence.proto/trogonai/session/sessions/v1/assistant_message_failed.proto#L11-L13: validate failure facts before persistence.#!/bin/bash set -euo pipefail rg -n "LEGACY_REQUIRED" proto/trogonai/session/sessions/v1 rg -n "SessionEvent|CheckpointProduced|AssistantMessageCompleted|AssistantMessageFailed|Serialize|Parse" .🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proto/trogonai/session/sessions/v1/artifact.proto` around lines 30 - 38, Replace LEGACY_REQUIRED with EXPLICIT for the artifact storage metadata and availability fields in proto/trogonai/session/sessions/v1/artifact.proto (lines 30-38 and 44-48), then add semantic validation before persistence. Apply the same presence and validation policy across proto/trogonai/session/sessions/v1/token_usage.proto (21-23), message.proto (26, 55), checkpoint.proto (13-19), execution_attempt_started.proto (13-16, 20-21), checkpoint_produced.proto (13-15, 19), session_forked.proto (12-14, 17), compacted.proto (16-20, 22), assistant_message_completed.proto (13-15), and assistant_message_failed.proto (11-13), validating each field group described by the corresponding event contract at construction, writer, or persistence boundaries; retain LEGACY_REQUIRED only where ADR#0035 explicitly requires wire compatibility.
37-48: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake degraded artifacts representable without stored-byte fields.
EXTERNAL_REFmeans only an external reference is kept, butstorage_refis part of the stored-byte key/description andcreated_atis marked required for the whole message, so a degraded artifact cannot round-trip unless synthetic storage metadata is invented. Makestorage_refandcreated_atopt-in/optional forEXTERNAL_REF, and require them forSTOREDplus validatestorage_refpresence for degraded references.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proto/trogonai/session/sessions/v1/artifact.proto` around lines 37 - 48, Update the artifact fields storage_ref and created_at to use optional presence rather than unconditional LEGACY_REQUIRED. Add validation keyed by ArtifactSourceAvailability: STORED artifacts must provide both fields, while EXTERNAL_REF artifacts may omit created_at but must provide storage_ref.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@proto/trogonai/session/sessions/v1/artifact.proto`:
- Line 48: Validate enum values before appending persisted events; field
presence alone does not reject zero-valued UNSPECIFIED values. In
proto/trogonai/session/sessions/v1/artifact.proto:48 require availability to be
STORED or EXTERNAL_REF; apply equivalent concrete-value validation to the
message role in message.proto:18, fork reason in session_forked.proto:17,
compaction trigger in compacted.proto:22, finish reason in
assistant_message_completed.proto:15, and failure reason in
assistant_message_failed.proto:13.
- Around line 49-50: Sanitize the value assigned to ArtifactMetadata.source_url
before it is persisted, rejecting or redacting URLs containing userinfo or
signed/authentication query tokens. Prefer recording a non-sensitive managed
storage or reference key when available, while preserving empty values for
artifacts without a source URL.
In `@proto/trogonai/session/sessions/v1/assistant_message_completed.proto`:
- Around line 13-15: Update the completed-event append/validation flow for the
CanonicalMessage field to require MESSAGE_ROLE_ASSISTANT before accepting the
event. Reject or avoid appending messages with user, system, or tool roles,
while preserving existing handling for valid assistant messages.
In `@proto/trogonai/session/sessions/v1/compacted.proto`:
- Around line 19-20: Validate the compaction range during event construction
before emitting the marker, ensuring from_sequence is not greater than
to_sequence. Reject reversed ranges immediately and prevent them from reaching
the event log, while preserving valid ranges.
---
Outside diff comments:
In `@proto/trogonai/session/sessions/v1/artifact.proto`:
- Around line 30-38: Replace LEGACY_REQUIRED with EXPLICIT for the artifact
storage metadata and availability fields in
proto/trogonai/session/sessions/v1/artifact.proto (lines 30-38 and 44-48), then
add semantic validation before persistence. Apply the same presence and
validation policy across proto/trogonai/session/sessions/v1/token_usage.proto
(21-23), message.proto (26, 55), checkpoint.proto (13-19),
execution_attempt_started.proto (13-16, 20-21), checkpoint_produced.proto
(13-15, 19), session_forked.proto (12-14, 17), compacted.proto (16-20, 22),
assistant_message_completed.proto (13-15), and assistant_message_failed.proto
(11-13), validating each field group described by the corresponding event
contract at construction, writer, or persistence boundaries; retain
LEGACY_REQUIRED only where ADR#0035 explicitly requires wire compatibility.
- Around line 37-48: Update the artifact fields storage_ref and created_at to
use optional presence rather than unconditional LEGACY_REQUIRED. Add validation
keyed by ArtifactSourceAvailability: STORED artifacts must provide both fields,
while EXTERNAL_REF artifacts may omit created_at but must provide storage_ref.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e67c476-5933-462f-969f-3e390921ecde
⛔ Files ignored due to path filters (34)
rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.artifact.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.artifact.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_completed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_completed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_failed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_failed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.checkpoint.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.checkpoint.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.checkpoint_produced.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.checkpoint_produced.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.compacted.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.compacted.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_attempt_started.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_attempt_started.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.file_changed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.file_changed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.mod.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.operation_outcome_recorded.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_forked.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_forked.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.system_notice_recorded.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.system_notice_recorded.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.token_usage.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.token_usage.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_denied.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_denied.rsis excluded by!**/gen/**
📒 Files selected for processing (15)
proto/trogonai/session/sessions/v1/artifact.protoproto/trogonai/session/sessions/v1/assistant_message_completed.protoproto/trogonai/session/sessions/v1/assistant_message_failed.protoproto/trogonai/session/sessions/v1/checkpoint.protoproto/trogonai/session/sessions/v1/checkpoint_produced.protoproto/trogonai/session/sessions/v1/compacted.protoproto/trogonai/session/sessions/v1/events.protoproto/trogonai/session/sessions/v1/execution_attempt_started.protoproto/trogonai/session/sessions/v1/file_changed.protoproto/trogonai/session/sessions/v1/message.protoproto/trogonai/session/sessions/v1/operation_outcome_recorded.protoproto/trogonai/session/sessions/v1/session_forked.protoproto/trogonai/session/sessions/v1/system_notice_recorded.protoproto/trogonai/session/sessions/v1/token_usage.protoproto/trogonai/session/sessions/v1/tool_call_denied.proto
🚧 Files skipped from review as they are similar to previous changes (1)
- proto/trogonai/session/sessions/v1/operation_outcome_recorded.proto
A multi-lens critic pass over the live model surfaced the tool-call/operation-ledger join, Claude's pause_turn/refusal stop reasons, a first-class plan/todo fact, and a per-branch required-ness fix for degraded artifacts; recording each keeps audit, cost, and plan projections deterministic folds of the log. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Second convergence round found only consistency and fold-correctness refinements, no new domain: partial-turn cost on AssistantMessageFailed, explanatory detail on the failure-only ExecutionAttemptEnded, an intentional-detach reason, and the artifact claim-check moved onto the shared Digest value object used everywhere else. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Third convergence round collapsed to a single required-presence annotation plus naming parity, no new domain: size_bytes required-ness, a generic artifact_ref arm name, and timeout parity on the assistant failure enum. The event data model is converged against all nine research products and the platform prior art. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…k in glossary Pin one precise meaning per term so a wider audience reads the session relationships the same way: the entity is a child session (a full session, one run of an agent), 'subagent' is only its informal synonym, and delegation/cascade-policy/fork are named distinctly rather than collapsed into 'sub'. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/glossary/cascade-policy.md`:
- Around line 9-12: Update the CascadePolicy glossary text to distinguish the
protobuf wire default, CASCADE_POLICY_UNSPECIFIED, from the application-level
safe default CASCADE_ON_PARENT_TERMINAL. In the command-layer handling of
cascade policy, explicitly record CASCADE_ON_PARENT_TERMINAL when the policy is
unspecified rather than relying on an omitted enum field.
In `@docs/glossary/fork.md`:
- Around line 9-12: Update the fork definition in the glossary to remove the
claim that forks never reference the source ID. State that a fork has its own
identity while persisting or retaining the source session reference used to
compose the shared prefix during replay.
In `@proto/trogonai/session/sessions/v1/artifact.proto`:
- Around line 44-47: Update ArtifactRecorded validation before appending
artifact events to require the source oneof to be set, rejecting records where
both stored and external are unset while preserving valid StoredArtifact and
ExternalArtifact records.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 45ef69a9-b431-4bb2-91b2-99d42830779f
⛔ Files ignored due to path filters (37)
rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.artifact.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.artifact.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.artifact.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.artifact.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_completed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_completed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_failed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.assistant_message_failed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.events.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_attempt_ended.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.execution_attempt_ended.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.message.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.mod.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_forked.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_forked.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_rewound.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.session_rewound.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.subagent_detached.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.subagent_detached.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.system_notice_recorded.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.system_notice_recorded.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.todo_updated.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.todo_updated.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.token_usage.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.token_usage.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_failed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_failed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_requested.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1.tool_call_requested.rsis excluded by!**/gen/**
📒 Files selected for processing (21)
docs/glossary/cascade-policy.mddocs/glossary/child-session.mddocs/glossary/delegation.mddocs/glossary/fork.mddocs/glossary/index.mddocs/glossary/subagent.mdproto/trogonai/session/sessions/v1/artifact.protoproto/trogonai/session/sessions/v1/assistant_message_completed.protoproto/trogonai/session/sessions/v1/assistant_message_failed.protoproto/trogonai/session/sessions/v1/events.protoproto/trogonai/session/sessions/v1/execution_attempt_ended.protoproto/trogonai/session/sessions/v1/message.protoproto/trogonai/session/sessions/v1/session_forked.protoproto/trogonai/session/sessions/v1/session_rewound.protoproto/trogonai/session/sessions/v1/subagent_detached.protoproto/trogonai/session/sessions/v1/system_notice_recorded.protoproto/trogonai/session/sessions/v1/todo_updated.protoproto/trogonai/session/sessions/v1/token_usage.protoproto/trogonai/session/sessions/v1/tool_call.protoproto/trogonai/session/sessions/v1/tool_call_failed.protoproto/trogonai/session/sessions/v1/tool_call_requested.proto
🚧 Files skipped from review as they are similar to previous changes (8)
- proto/trogonai/session/sessions/v1/subagent_detached.proto
- proto/trogonai/session/sessions/v1/tool_call.proto
- proto/trogonai/session/sessions/v1/system_notice_recorded.proto
- proto/trogonai/session/sessions/v1/execution_attempt_ended.proto
- proto/trogonai/session/sessions/v1/session_forked.proto
- proto/trogonai/session/sessions/v1/token_usage.proto
- proto/trogonai/session/sessions/v1/events.proto
- proto/trogonai/session/sessions/v1/message.proto
The data model now speaks one precise vocabulary matching the glossary: SubagentLinked->ParentLinked, SubagentParentTerminated->ParentTerminated, SubagentDetached->DelegationDetached, so the entity is a child session and 'subagent' stays only an informal synonym in prose. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…e in v1alpha1 Physical JetStream positions change on restore while event bytes do not, so durable payloads now reference fold-derived SessionOrdinal positions, fork mints a self-contained session instead of importing the source aggregate, and the alpha package suffix stops a draft-dependent contract from reading as frozen. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
The prior facet text promised dedup, fork, and detach semantics the substrate cannot deliver yet; the record now states the decided contract, the fold rules, and the substrate obligations explicitly. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
No workflow guarded general protobuf changes, so formatting and codegen drift could only be caught locally. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
proto/trogonai/session/sessions/v1alpha1/assistant_message_failed.proto (1)
28-40: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject zero-valued required enum fields before appending session events.
LEGACY_REQUIREDonly records that the enum field was present; it still accepts*_UNSPECIFIED = 0values. Validate explicitly named enum values before decoding/appending:
AssistantMessageFailed.reason(lines 28-40): onlyINTERRUPTED,CANCELLED,ERROR,TIMEOUT.ExecutionAttemptEnded.outcome(lines 24-30): onlyFAILED,CANCELLED,TERMINATED.ParentLinked.cascade_policy(line 20): onlyCASCADE_ON_PARENT_TERMINALorINDEPENDENT.ParentTerminated.cause(lines 23-28): onlyCLOSED,CANCELLED,FAILED,HIDDEN.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proto/trogonai/session/sessions/v1alpha1/assistant_message_failed.proto` around lines 28 - 40, Reject zero-valued unspecified enums before decoding or appending session events: validate AssistantMessageFailed.reason to allow only INTERRUPTED, CANCELLED, ERROR, or TIMEOUT; execution_attempt_ended.proto:24-30 validate ExecutionAttemptEnded.outcome for only FAILED, CANCELLED, or TERMINATED; parent_linked.proto:20 validate ParentLinked.cascade_policy for only CASCADE_ON_PARENT_TERMINAL or INDEPENDENT; and parent_terminated.proto:23-28 validate ParentTerminated.cause for only CLOSED, CANCELLED, FAILED, or HIDDEN, rather than relying solely on LEGACY_REQUIRED.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci-proto.yml:
- Around line 6-8: Update the pull_request trigger in the CI workflow to include
the edited activity alongside opened, synchronize, and reopened, so retargeting
a PR to main reruns validation.
In `@docs/adr/0035-session-store-decider-aggregate.md`:
- Around line 275-283: Update the fold deduplication contract around evolve,
projections, and snapshot restoration to retain enough per-stream event-ID state
to prevent any duplicate from being applied again on an unbounded log. Replace
the bounded seen-key requirement with complete ID retention, a provably safe
ordinal/watermark scheme, or an explicit retention horizon that prevents old IDs
from reappearing after restore.
- Around line 257-265: Update the identity and dedup contract to namespace
derived event IDs across the shared SESSION_EVENTS stream, incorporating the
session/subject key and event or command type into the UUIDv5 input alongside
the idempotency key and batch index; alternatively, explicitly require globally
unique idempotency keys for the entire stream.
In `@proto/trogonai/session/sessions/v1alpha1/artifact.proto`:
- Around line 69-70: Update the ExternalArtifact comment to describe the source
bytes as not durably stored rather than claiming they were never fetched, so
fetched_at and content_digest remain valid evidence of attempted or transient
fetches.
In `@proto/trogonai/session/sessions/v1alpha1/delegation_dispatched.proto`:
- Line 21: Enforce exact cascade_policy equality between DelegationDispatched
and ParentLinked when correlating or writing records by operation_id. Update the
logic associated with
proto/trogonai/session/sessions/v1alpha1/delegation_dispatched.proto:21 and
proto/trogonai/session/sessions/v1alpha1/parent_linked.proto:20-23 so the
child-side value is validated against the parent dispatch value, rejecting
mismatches before lineage or cascade decisions; both sites require coordinated
handling.
In `@proto/trogonai/session/sessions/v1alpha1/execution_attempt_started.proto`:
- Around line 22-25: Update the session-store validation for restarted attempts
using previous_attempt_id: require it to reference the immediately preceding
attempt ID, rejecting missing or nonmatching lineage; alternatively, distinguish
restart lineage with prev_attempt_id while representing initial attempts
explicitly. Preserve valid initial-attempt handling and existing attempt_number
behavior.
In `@proto/trogonai/session/sessions/v1alpha1/session_failed.proto`:
- Around line 10-11: Update the detail field in SessionFailed to use optional
presence semantics, matching SessionCancelled.detail, so events without failure
details remain valid and the documented empty-when-absent behavior is preserved.
---
Outside diff comments:
In `@proto/trogonai/session/sessions/v1alpha1/assistant_message_failed.proto`:
- Around line 28-40: Reject zero-valued unspecified enums before decoding or
appending session events: validate AssistantMessageFailed.reason to allow only
INTERRUPTED, CANCELLED, ERROR, or TIMEOUT; execution_attempt_ended.proto:24-30
validate ExecutionAttemptEnded.outcome for only FAILED, CANCELLED, or
TERMINATED; parent_linked.proto:20 validate ParentLinked.cascade_policy for only
CASCADE_ON_PARENT_TERMINAL or INDEPENDENT; and parent_terminated.proto:23-28
validate ParentTerminated.cause for only CLOSED, CANCELLED, FAILED, or HIDDEN,
rather than relying solely on LEGACY_REQUIRED.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 68f9f0a7-f8eb-4b51-bb35-9608b8a0ab0c
⛔ Files ignored due to path filters (114)
rsworkspace/crates/platform/trogonai-proto/src/gen/mod.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.artifact.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.artifact.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.artifact.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.artifact.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.artifact_erased.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.artifact_erased.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.artifact_recorded.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.artifact_recorded.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.assistant_message_completed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.assistant_message_completed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.assistant_message_failed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.assistant_message_failed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.assistant_message_started.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.assistant_message_started.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.cascade_policy.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.checkpoint.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.checkpoint.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.checkpoint_produced.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.checkpoint_produced.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.compacted.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.compacted.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.delegation_detached.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.delegation_detached.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.delegation_dispatched.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.delegation_dispatched.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.digest.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.digest.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.events.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.events.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.events.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.events.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.execution_attempt_ended.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.execution_attempt_ended.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.execution_attempt_ready.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.execution_attempt_ready.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.execution_attempt_started.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.execution_attempt_started.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.execution_plan.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.execution_plan.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.external_delegation_dispatched.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.external_delegation_dispatched.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.file_change.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.file_changed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.file_changed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.message.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.message.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.message.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.message.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.mod.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.operation_cancellation_requested.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.operation_cancellation_requested.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.operation_outcome_recorded.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.operation_outcome_recorded.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.operation_outcome_recorded.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.operation_outcome_recorded.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.operation_reserved.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.operation_reserved.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.parent_detached.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.parent_detached.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.parent_history_invalidated.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.parent_history_invalidated.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.parent_linked.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.parent_linked.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.parent_terminated.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.parent_terminated.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.redaction_applied.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.redaction_applied.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_archived.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_archived.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_cancelled.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_cancelled.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_closed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_closed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_failed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_failed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_forked.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_forked.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_hidden.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_hidden.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_ordinal.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_ordinal.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_renamed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_renamed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_rewound.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_rewound.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_started.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_started.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_unarchived.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.session_unarchived.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.system_notice_recorded.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.system_notice_recorded.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.todo_updated.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.todo_updated.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.token_usage.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.token_usage.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call.__oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call.__view_oneof.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_approved.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_approved.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_completed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_completed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_denied.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_denied.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_failed.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_failed.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_requested.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_requested.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_started.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.tool_call_started.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.user_message_recorded.__view.rsis excluded by!**/gen/**rsworkspace/crates/platform/trogonai-proto/src/gen/trogonai.session.sessions.v1alpha1.user_message_recorded.rsis excluded by!**/gen/**
📒 Files selected for processing (57)
.github/workflows/ci-proto.ymldocs/adr/0035-session-store-decider-aggregate.mddocs/glossary/cascade-policy.mddocs/glossary/fork.mddocs/research/session-store/index.mdproto/trogonai/session/sessions/v1alpha1/artifact.protoproto/trogonai/session/sessions/v1alpha1/artifact_erased.protoproto/trogonai/session/sessions/v1alpha1/artifact_recorded.protoproto/trogonai/session/sessions/v1alpha1/assistant_message_completed.protoproto/trogonai/session/sessions/v1alpha1/assistant_message_failed.protoproto/trogonai/session/sessions/v1alpha1/assistant_message_started.protoproto/trogonai/session/sessions/v1alpha1/cascade_policy.protoproto/trogonai/session/sessions/v1alpha1/checkpoint.protoproto/trogonai/session/sessions/v1alpha1/checkpoint_produced.protoproto/trogonai/session/sessions/v1alpha1/compacted.protoproto/trogonai/session/sessions/v1alpha1/delegation_detached.protoproto/trogonai/session/sessions/v1alpha1/delegation_dispatched.protoproto/trogonai/session/sessions/v1alpha1/digest.protoproto/trogonai/session/sessions/v1alpha1/events.protoproto/trogonai/session/sessions/v1alpha1/execution_attempt_ended.protoproto/trogonai/session/sessions/v1alpha1/execution_attempt_ready.protoproto/trogonai/session/sessions/v1alpha1/execution_attempt_started.protoproto/trogonai/session/sessions/v1alpha1/execution_plan.protoproto/trogonai/session/sessions/v1alpha1/external_delegation_dispatched.protoproto/trogonai/session/sessions/v1alpha1/file_change.protoproto/trogonai/session/sessions/v1alpha1/file_changed.protoproto/trogonai/session/sessions/v1alpha1/message.protoproto/trogonai/session/sessions/v1alpha1/operation_cancellation_requested.protoproto/trogonai/session/sessions/v1alpha1/operation_outcome_recorded.protoproto/trogonai/session/sessions/v1alpha1/operation_reserved.protoproto/trogonai/session/sessions/v1alpha1/parent_detached.protoproto/trogonai/session/sessions/v1alpha1/parent_history_invalidated.protoproto/trogonai/session/sessions/v1alpha1/parent_linked.protoproto/trogonai/session/sessions/v1alpha1/parent_terminated.protoproto/trogonai/session/sessions/v1alpha1/redaction_applied.protoproto/trogonai/session/sessions/v1alpha1/session_archived.protoproto/trogonai/session/sessions/v1alpha1/session_cancelled.protoproto/trogonai/session/sessions/v1alpha1/session_closed.protoproto/trogonai/session/sessions/v1alpha1/session_failed.protoproto/trogonai/session/sessions/v1alpha1/session_forked.protoproto/trogonai/session/sessions/v1alpha1/session_hidden.protoproto/trogonai/session/sessions/v1alpha1/session_ordinal.protoproto/trogonai/session/sessions/v1alpha1/session_renamed.protoproto/trogonai/session/sessions/v1alpha1/session_rewound.protoproto/trogonai/session/sessions/v1alpha1/session_started.protoproto/trogonai/session/sessions/v1alpha1/session_unarchived.protoproto/trogonai/session/sessions/v1alpha1/system_notice_recorded.protoproto/trogonai/session/sessions/v1alpha1/todo_updated.protoproto/trogonai/session/sessions/v1alpha1/token_usage.protoproto/trogonai/session/sessions/v1alpha1/tool_call.protoproto/trogonai/session/sessions/v1alpha1/tool_call_approved.protoproto/trogonai/session/sessions/v1alpha1/tool_call_completed.protoproto/trogonai/session/sessions/v1alpha1/tool_call_denied.protoproto/trogonai/session/sessions/v1alpha1/tool_call_failed.protoproto/trogonai/session/sessions/v1alpha1/tool_call_requested.protoproto/trogonai/session/sessions/v1alpha1/tool_call_started.protoproto/trogonai/session/sessions/v1alpha1/user_message_recorded.proto
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/glossary/fork.md
- docs/glossary/cascade-policy.md
Comment on lines
+6
to
+8
| pull_request: | ||
| types: [opened, synchronize, reopened] | ||
| branches: [main] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files =="
git ls-files '.github/workflows/*' || true
echo
echo "== ci-proto.yml =="
cat -n .github/workflows/ci-proto.yml
echo
echo "== trigger type tests in repo =="
rg -n "pull_request|pull_request_target|pull_request_review|pull_request_target:|paths:\"|edited|opened|synchronize|rerequested|labeled|unlabeled|reopened|ready_for_review|review_requested|closed" .github/workflows || trueRepository: TrogonStack/trogonai
Length of output: 6150
🌐 Web query:
GitHub Actions pull_request events edited activity types opened synchronize reopened documentation
💡 Result:
In GitHub Actions, the pull_request event triggers workflows based on specific activity types [1]. By default, if no activity types are specified in your workflow configuration, the pull_request event only triggers on opened, synchronize, or reopened activity [1][2]. To trigger workflows on other activity types, such as edited, you must explicitly define them using the types keyword in your workflow file [1]. Key Activity Types for pull_request: 1. opened: Triggered when a new pull request is created [1]. 2. synchronize: Triggered when a pull request's head branch is updated (e.g., new commits are pushed to the head branch) [1][3]. 3. reopened: Triggered when a previously closed pull request is reopened [1]. 4. edited: Triggered when the title or body of a pull request is edited, or when the base branch of the pull request is changed [4][3]. Configuration Example: To include edited alongside the default types, configure your workflow as follows: on: pull_request: types: [opened, synchronize, reopened, edited] Important Considerations: * The edited activity type is broad and triggers for changes to the title, body, or base branch [4][3]. If you need to restrict a job to only run on specific types of edits (e.g., only when the base branch changes), you can use conditional expressions (if: github.event.action == 'edited' && github.event.changes.base) within your workflow steps or jobs [4][3]. * Security note: Workflows using the GITHUB_TOKEN that are triggered by opened, synchronize, or reopened activities on pull requests created by automation may require manual approval depending on the repository settings [2]. Other activity types like edited do not create these specific approval-required workflow runs [2].
Citations:
- 1: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows
- 2: https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/trigger-a-workflow
- 3: Action does not get triggered for last commit when base branch is changed actions/runner#980
- 4: https://stackoverflow.com/questions/79143203/is-it-possible-to-make-github-actions-ignore-edited-activity-when-its-only-pr
🌐 Web query:
GitHub Actions pull_request event merges code when tests run pull_request workflow default branch origin/main
💡 Result:
When a GitHub Actions workflow is triggered by the pull_request event, it does not automatically merge your code into the default branch (e.g., origin/main) [1]. Instead, GitHub creates a temporary "merge commit" that represents what the pull request would look like if it were merged into the target branch [1][2][3]. By default, when you use the actions/checkout action within a pull_request workflow, it checks out this temporary merge commit rather than the head of your pull request branch [1][3][4]. This allows your CI tests to run against the code as it would exist after a successful merge, ensuring that the changes are compatible with the current state of the target branch [1][4]. Key points regarding this behavior: - Temporary Merge: The merge commit is transient and used solely for the purpose of running the workflow [1][2]. It does not modify your actual repository branches [1]. - GITHUB_SHA: When using the pull_request event, the GITHUB_SHA environment variable refers to this temporary merge commit, not the latest commit on your feature branch [2][4]. - pull_request vs. pull_request_target: The pull_request event runs code from the pull request itself (with restricted permissions for forks) [5][6]. In contrast, the pull_request_target event runs using the workflow definition from the target branch (e.g., origin/main), which allows it to access secrets and have write permissions, but requires caution as it can execute code from the target branch on the pull request context [5][6]. - Triggering on Merge: If you want to run a workflow specifically after a pull request has been merged, you must use the closed activity type of the pull_request event and check the merged state in a conditional step [1][7][8]. Example check for a merged PR: if: github.event.pull_request.merged == true [7][8]
Citations:
- 1: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows
- 2: GITHUB_SHA on PRs refers to the merge commit, not the actual commit which triggered the PR github/docs#15302
- 3: https://stackoverflow.com/questions/63323498/during-a-pr-action-on-github-what-version-of-the-code-is-actually-run
- 4: Checkout the test merge of a PR, rather than the head of the PR branch actions/checkout#15
- 5: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target
- 6: https://stackoverflow.com/questions/74957218/what-is-the-difference-between-pull-request-and-pull-request-target-event-in-git
- 7: https://stackoverflow.com/questions/60710209/trigger-github-actions-only-when-pr-is-merged
- 8: https://graphite.com/guides/automating-post-merge-workflows-on-github
Run validation when a PR is retargeted to main.
A base-branch change is an edited PR activity, but it is excluded here. A proto PR retargeted to main will not rerun buf breaking against its new origin/main baseline. Add edited to the workflow trigger.
Proposed fix
pull_request:
- types: [opened, synchronize, reopened]
+ types: [opened, synchronize, reopened, edited]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pull_request: | |
| types: [opened, synchronize, reopened] | |
| branches: [main] | |
| pull_request: | |
| types: [opened, synchronize, reopened, edited] | |
| branches: [main] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci-proto.yml around lines 6 - 8, Update the pull_request
trigger in the CI workflow to include the edited activity alongside opened,
synchronize, and reopened, so retargeting a PR to main reruns validation.
Comment on lines
+257
to
+265
| **Identity and dedup contract.** Commands are processed at-least-once -- a NATS | ||
| processor redelivers a command after a crash before its ack -- so every command | ||
| carries a caller-supplied idempotency key, stable across redelivery, and no | ||
| domain payload gains a separate identity field of its own. The runtime derives | ||
| each appended event's envelope `Event.id` deterministically: UUIDv5 over | ||
| `(command idempotency key, index of the event within the decision's batch)`. A | ||
| redelivered command therefore reproduces byte-identical event ids on retry, | ||
| while distinct events within one multi-event batch (a `[SessionStarted, | ||
| SessionForked]` fork, say) stay distinct -- no batch aliasing. This is what makes |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate ADR =="
fd -a '0035-session-store-decider-aggregate\.md$' . || true
echo "== Relevant lines =="
for f in $(fd '0035-session-store-decider-aggregate\.md$' .); do
echo "--- $f ---"
wc -l "$f"
sed -n '230,290p' "$f" | nl -ba -v230
done
echo "== Search event id/idempotency/msg-id references =="
rg -n "Event\.id|idempotency|Md-Id|Nats-Msg-Id|stream_subject|batch_index|UUIDv5|dedup|duplicate" docs/adr/0035-session-store-decider-aggregate.md docs 2>/dev/null | head -n 200
echo "== Search code/runtime for deterministic event id derivation and Nats-Msg-Id usage =="
rg -n "uuidv5|UUIDv5|event\.id|Nats-Msg-Id|NatsMsgId|idempotency|idempotency key|SessionStarted|SessionForked" . --glob '!node_modules' --glob '!dist' --glob '!build' 2>/dev/null | head -n 240Repository: TrogonStack/trogonai
Length of output: 419
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ADR relevant lines =="
sed -n '230,290p' docs/adr/0035-session-store-decider-aggregate.md | awk '{printf "%6d\t%s\n", NR+229, $0}'
echo "== ADR search =="
rg -n "Event\.id|idempotency|Md-Id|Nats-Msg-Id|stream_subject|batch_index|UUIDv5|dedup|duplicate|SessionStarted|SessionForked|one physical|physical JetStream|JetStream stream" docs/adr/0035-session-store-decider-aggregate.md docs/adr 2>/dev/null | head -n 220
echo "== runtime search =="
rg -n "uuidv5|UUIDv5|event\.id|Nats-Msg-Id|NatsMsgId|idempotency|idempotency key|SessionStarted|SessionForked|Nats.*Msg.*Id|msgId|Msg-Id" . --glob '!node_modules' --glob '!dist' --glob '!build' 2>/dev/null | head -n 240Repository: TrogonStack/trogonai
Length of output: 50376
🌐 Web query:
NATS JetStream duplicate message detection Nats-Msg-Id documentation
💡 Result:
In NATS JetStream, the Nats-Msg-Id header is a client-defined unique identifier used to enable idempotent message publishing and duplicate message detection [1][2][3]. When a message is published to a JetStream stream containing the Nats-Msg-Id header, the JetStream server tracks this ID for a configurable duration known as the DuplicateWindow [1][2][4]. If a message with an identical ID is received within this window, the server recognizes it as a duplicate, ignores the write, and does not store the duplicate message [1][5]. Key details regarding this mechanism include: 1. Identification: The server only consults the Nats-Msg-Id header value to determine duplication, not the message body [1]. 2. Duplicate Window: This window is configured at the stream level and represents the time duration during which duplicate messages are tracked [1][5]. The default window is 2 minutes [1]. 3. Exactly-Once Semantics: By combining message deduplication (via Nats-Msg-Id) with client-side acknowledgments (ack), JetStream supports exactly-once publication and consumption [1]. 4. Client SDK Usage: Modern NATS clients typically expose this via publish options (e.g., setting msgID in publish options), which automatically handles the inclusion of the header [6][7]. For scenarios requiring optimistic concurrency control, NATS also provides the Nats-Expected-Last-Msg-Id header, which allows a producer to assert that the last message ID on the stream matches a specific value before allowing a new publish to succeed [2][3].
Citations:
- 1: https://docs.nats.io/using-nats/developer/develop_jetstream/model_deep_dive
- 2: https://docs.nats.io/nats-concepts/jetstream/headers.md
- 3: https://docs.nats.io/nats-concepts/jetstream/headers
- 4: https://nats.io/blog/new-per-subject-discard-policy/
- 5: https://docs.nats.io/nats-concepts/jetstream/streams
- 6: https://docs.nats.io/using-nats/developer/develop_jetstream/publish
- 7: https://pkg.go.dev/github.com/nats-io/nats.go/jetstream
Namespace event IDs before publishing them with Nats-Msg-Id.
Event.id is only UUIDv5(command idempotency key, batch_index), but every session subject appends to the shared SESSION_EVENTS JetStream stream. Reusing a valid key/index pair for a different session or event type makes NATS treat the distinct event as a duplicate and prevents it from being appended. Include the subject/session key and event/command type in the UUIDv5 input, or enforce globally unique idempotency keys for the entire stream.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0035-session-store-decider-aggregate.md` around lines 257 - 265,
Update the identity and dedup contract to namespace derived event IDs across the
shared SESSION_EVENTS stream, incorporating the session/subject key and event or
command type into the UUIDv5 input alongside the idempotency key and batch
index; alternatively, explicitly require globally unique idempotency keys for
the entire stream.
Source: MCP tools
Comment on lines
+275
to
+283
| Publish dedup closes the write side: the append sets the NATS header | ||
| `Nats-Msg-Id` to the event id, and a duplicate acknowledgement inside the | ||
| JetStream dedup window is idempotent success, not an append error. Fold dedup | ||
| closes the read side: `evolve` (native and WASM) must receive the envelope event | ||
| id, and the fold and every projection collapse events with an already-seen id, | ||
| with snapshots persisting the bounded seen-key state they need to keep doing so | ||
| across a restore. Beyond the dedup window, a guarded (`At`) command's retry | ||
| re-replays and no-ops on its idempotency key as before; an `Any` fact past the | ||
| window relies on this reader-side collapse by identical id. `Trogon-Correlation-Id` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files relevant to ADR:"
git ls-files | rg '(^docs/adr/0035-session-store-decider-aggregate\.md$|session-store|dedup|fold|projection|snapshot|nats|jetstream)' || true
echo
echo "Target excerpt:"
sed -n '240,305p' docs/adr/0035-session-store-decider-aggregate.md | cat -n
echo
echo "Search for dedup/fold/snapshot retention language:"
rg -n -C 3 'dedup|duplicate|snapshot|seen-key|bounded|At|Any|Projection|ID|idempot|Trogon-Correlation-Id|Nats-Msg-Id' docs/adr/0035-session-store-decider-aggregate.mdRepository: TrogonStack/trogonai
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Target ADR lines 256-290:"
sed -n '256,290p' docs/adr/0035-session-store-decider-aggregate.md | cat -n
echo
echo "Relevant ADR sections around stream semantics/dedup:"
sed -n '1,120p' docs/adr/0035-session-store-decider-aggregate.md | cat -n
sed -n '200,330p' docs/adr/0035-session-store-decider-aggregate.md | cat -n
echo
echo "Search for dedup/window/stream retention terms in target ADR:"
rg -n -C 2 'duplicate|dedup|window|max_age|max_msgs|bounded|stream|JetStream|Nats-Msg-Id|Trogon-Correlation-Id|restore|snapshot|idempot|projection|fold' docs/adr/0035-session-store-decider-aggregate.md | head -n 220Repository: TrogonStack/trogonai
Length of output: 36567
🌐 Web query:
NATS JetStream duplicate detection message-id duplicate only within duplicate window documentation
💡 Result:
NATS JetStream provides message deduplication on the publishing side to support idempotent writes, ensuring that duplicate messages with the same ID are not stored in a stream [1][2]. How it works: 1. Nats-Msg-Id Header: To enable deduplication, a publisher must attach a unique identifier to the message using the Nats-Msg-Id header [3][2]. 2. Duplicate Window: The NATS server tracks these identifiers for a configurable duration known as the Duplicate Window [1][4]. 3. Detection: When a message with a Nats-Msg-Id is published, the JetStream server checks if it has already recorded the same ID within the active Duplicate Window [1][2]. 4. Behavior: If a duplicate ID is detected within this window, the server rejects the subsequent publish (or ignores it), ensuring that only the first message is stored [1][5]. Key Details: - Default Window: The default duration for the duplicate window is 2 minutes [1]. - Configuration: This window can be customized when creating or updating a stream by setting the DuplicateWindow property (expressed in nanoseconds) [4][6]. For CLI-based configurations, this is typically handled via the --dupe-window flag [1]. - Limitations: Deduplication is strictly tied to this sliding time window [4]. If a duplicate message is published after the Duplicate Window for the original message has expired, the server will treat it as a new, unique message [1][2]. The documentation cautions against configuring excessively large windows due to potential performance impacts [1]. It is important to note that this mechanism is specifically for preventing duplicate storage during the publishing phase. JetStream achieves true "exactly-once" delivery semantics by combining this publishing-side deduplication with a double-acknowledgment mechanism (using AckSync) on the consumer side [2][7].
Citations:
- 1: https://docs.nats.io/using-nats/developer/develop_jetstream/model_deep_dive
- 2: https://docs.nats.io/nats-concepts/jetstream/key-value-store/kv_walkthrough?ask=How+does+JetStream+provide+exactly-once+delivery+semantics%3F
- 3: https://docs.nats.io/nats-concepts/jetstream/headers
- 4: https://docs.nats.io/nats-concepts/jetstream/streams
- 5: https://docs.nats.io/using-nats/developer/develop_jetstream/model_deep_dive.md
- 6: https://docs.nats.io/nats-concepts/jetstream/streams.md
- 7: https://docs.nats.io/nats-concepts/jetstream
Define complete retention for fold deduplication IDs.
This contract keeps the keep-forever log invariant while relying on reader-side dedup beyond JetStream’s finite Nats-Msg-Id duplicate window. A “bounded seen-key” snapshot after restore would let a late duplicate ID beyond that window be applied again, corrupting folds. Specify complete per-stream ID retention, a provably safe ordinal/watermark scheme, or an explicit retention horizon that prevents old duplicates from reappearing on unbounded logs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0035-session-store-decider-aggregate.md` around lines 275 - 283,
Update the fold deduplication contract around evolve, projections, and snapshot
restoration to retain enough per-stream event-ID state to prevent any duplicate
from being applied again on an unbounded log. Replace the bounded seen-key
requirement with complete ID retention, a provably safe ordinal/watermark
scheme, or an explicit retention horizon that prevents old IDs from reappearing
after restore.
Source: MCP tools
Comment on lines
+69
to
+70
| // ExternalArtifact is the degraded source arm for an artifact whose bytes were | ||
| // never fetched; only a reference to the source is kept. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Clarify ExternalArtifact fetch semantics.
The comment says the bytes were “never fetched,” while fetched_at and content_digest model attempted or transient fetches. Describe the source as “not durably stored” or split these states so consumers do not treat valid fetch evidence as impossible.
Proposed wording
-// ExternalArtifact is the degraded source arm for an artifact whose bytes were
-// never fetched; only a reference to the source is kept.
+// ExternalArtifact is the degraded source arm for an artifact whose bytes were
+// not durably stored; the source reference and optional transient fetch evidence
+// are retained.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // ExternalArtifact is the degraded source arm for an artifact whose bytes were | |
| // never fetched; only a reference to the source is kept. | |
| // ExternalArtifact is the degraded source arm for an artifact whose bytes were | |
| // not durably stored; the source reference and optional transient fetch evidence | |
| // are retained. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@proto/trogonai/session/sessions/v1alpha1/artifact.proto` around lines 69 -
70, Update the ExternalArtifact comment to describe the source bytes as not
durably stored rather than claiming they were never fetched, so fetched_at and
content_digest remain valid evidence of attempted or transient fetches.
| string session_id = 1 [features.field_presence = LEGACY_REQUIRED]; | ||
| string operation_id = 2 [features.field_presence = LEGACY_REQUIRED]; | ||
| string child_session_id = 3 [features.field_presence = LEGACY_REQUIRED]; | ||
| CascadePolicy cascade_policy = 5 [features.field_presence = LEGACY_REQUIRED]; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Relevant proto files:"
for f in proto/trogonai/session/sessions/v1alpha1/delegation_dispatched.proto proto/trogonai/session/sessions/v1alpha1/parent_linked.proto; do
if [ -f "$f" ]; then
echo "--- $f"
nl -ba "$f" | sed -n '1,90p'
else
echo "MISSING $f"
fi
done
echo
echo "Search for DelegationDispatched / ParentLinked / cascade_policy / operation_id usages:"
rg -n "DelegationDispatched|ParentLinked|cascade_policy|operation_id|DelegationDispatch|ParentLinked|parent_linked|delegation_dispatched" -S . \
--glob '!node_modules' --glob '!dist' --glob '!build' --glob '!*.lock' \
| head -n 250Repository: TrogonStack/trogonai
Length of output: 293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Relevant proto files:"
for f in proto/trogonai/session/sessions/v1alpha1/delegation_dispatched.proto proto/trogonai/session/sessions/v1alpha1/parent_linked.proto; do
if [ -f "$f" ]; then
echo "--- $f"
awk '{printf "%4d\t%s\n", NR, $0}' "$f" | sed -n '1,120p'
else
echo "MISSING $f"
fi
done
echo
echo "Search for DelegationDispatched / ParentLinked / cascade_policy / operation_id usages:"
rg -n "DelegationDispatched|ParentLinked|cascade_policy|operation_id|DelegationDispatch|ParentLinked|parent_linked|delegation_dispatched" -S . \
--glob '!node_modules' --glob '!dist' --glob '!build' --glob '!*.lock' \
| sed -n '1,300p'Repository: TrogonStack/trogonai
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Source files under non-generated workspace:"
git ls-files ':!:rsworkspace/crates/platform/trogonai-proto/src/gen/**' \
':!:*.pb.go' \
':!:*.pyc' \
':!:node_modules/**' \
':!:dist/**' \
':!:build/**' \
':!:*.lock' | sed -n '1,200p'
echo
echo "Search dispatch/creation/cascade validation in source, excluding generated proto:"
files=$(git ls-files ':!:rsworkspace/crates/platform/trogonai-proto/src/gen/**' ':!:*.pb.go' ':!:*.pyc' ':!:node_modules/**' ':!:dist/**' ':!:build/**' ':!:*.lock')
echo "$files" | xargs rg -n "DispatchDelegation|CreateChildSession|CreateChild|ParentLinked|DelegationDispatched|cascade_policy|cascadePolicy|ParentLinked|DETERMINED_CASCADE|CASCADE_ON_PARENT_TERMINAL|INDEPENDENT" \
--glob '!*.pb.go' --glob '!*.pyc' --glob '!*.lock' \
2>/dev/null \
| sed -n '1,240p'Repository: TrogonStack/trogonai
Length of output: 15399
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate cascade_policy proto and inspect it:"
for f in proto/trogonai/session/sessions/v1alpha1/cascade_policy.proto docs/adr/0035-session-store-decider-aggregate.md; do
if [ -f "$f" ]; then
echo "--- $f"
awk '{printf "%4d\t%s\n", NR, $0}' "$f" | sed -n '1,140p'
else
echo "MISSING $f"
fi
done
echo
echo "Potential source candidates containing delegate/session creation functions:"
git ls-files ':!:rsworkspace/crates/platform/trogonai-proto/src/gen/**' ':!:*.pb.go' ':!:node_modules/**' ':!:dist/**' ':!:build/**' | grep -E '\.(rs|go|ts|tsx|js|jsx)$' \
| head -n 80 \
| xargs awk 'FILENAME ~ /\.(rs|go|ts|tsx|js|jsx)$/ && /struct |pub fn |fn |class |function |case |match/ {print NR":"FILENAME":"$0}' \
| rg -n "delegate|Delegate|Dispatch|Session|ParentLinked|cascade_policy|cascadePolicy" \
| sed -n '1,240p'Repository: TrogonStack/trogonai
Length of output: 11396
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' EXIT
echo "Files containing exact identifiers, excluding auto-generated/generated-looking paths:"
rg -n --hidden -g '!rsworkspace/crates/platform/trogonai-proto/src/gen/**' -g '!vendor/**' -g '!third_party/**' \
"DispatchDelegation|CreateChildSession|CreateChild|ParentLinked|DelegationDispatched|cascade_policy|cascadePolicy|cascade_policy\.cascade_policy|cascade_policy ==|cascadePolicy ==|DETERMINED_CASCADE|CASCADE_POLICY_CASCADE_ON_PARENT_TERMINAL|CASCADE_POLICY_INDEPENDENT" . \
--glob '!*.pb.go' --glob '!*.pb.js' --glob '!*.proto' --glob '!*.lock' \
| sed -n '1,260p'
echo
echo "Inspect ADR sections around dispatch/rewind semantics:"
awk 'NR>=505 && NR<=590 {printf "%4d\t%s\n", NR, $0}' docs/adr/0035-session-store-decider-aggregate.md
awk 'NR>=740 && NR<=765 {printf "%4d\t%s\n", NR, $0}' docs/adr/0035-session-store-decider-aggregate.md
echo
echo "Inspect rewrite/proto files with cascade_policy for rewrite semantics:"
for f in proto/trogonai/session/sessions/v1alpha1/session_rewound.proto proto/trogonai/session/sessions/v1alpha1/parent_history_invalidated.proto; do
[ -f "$f" ] || continue
echo "--- $f"
awk '{printf "%4d\t%s\n", NR, $0}' "$f" | sed -n '1,90p'
doneRepository: TrogonStack/trogonai
Length of output: 17270
Enforce parent/child cascade-policy equality.
DelegationDispatched and ParentLinked are joined by operation_id, but ParentLinked has no source-of-truth field for the dispatch’s cascade_policy; both persist the same enum independently. Require the child-side value to exactly match the parent dispatch when writing/correlating these facts to avoid lineage/cascade decisions based on mismatched policy values.
📍 Affects 2 files
proto/trogonai/session/sessions/v1alpha1/delegation_dispatched.proto#L21-L21(this comment)proto/trogonai/session/sessions/v1alpha1/parent_linked.proto#L20-L23
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@proto/trogonai/session/sessions/v1alpha1/delegation_dispatched.proto` at line
21, Enforce exact cascade_policy equality between DelegationDispatched and
ParentLinked when correlating or writing records by operation_id. Update the
logic associated with
proto/trogonai/session/sessions/v1alpha1/delegation_dispatched.proto:21 and
proto/trogonai/session/sessions/v1alpha1/parent_linked.proto:20-23 so the
child-side value is validated against the parent dispatch value, rejecting
mismatches before lineage or cascade decisions; both sites require coordinated
handling.
Comment on lines
+22
to
+25
| uint64 attempt_number = 4 [features.field_presence = LEGACY_REQUIRED]; | ||
| string previous_attempt_id = 5; | ||
| Checkpoint restored_checkpoint = 6; | ||
| string resume_cursor = 7; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the proto definition and nearby usages/validators/comments.
printf '--- file exists ---\n'
git ls-files | rg 'proto/trogonai/session/sessions/v1alpha1/execution_attempt_started\.proto|execution_attempt_started' || true
printf '\n--- proto definition ---\n'
sed -n '1,120p' proto/trogonai/session/sessions/v1alpha1/execution_attempt_started.proto 2>/dev/null || true
printf '\n--- related proto files names ---\n'
git ls-files 'proto/trogonai/session/sessions/v1alpha1/*' | xargs -r -n1 basename | sort
printf '\n--- search previous_attempt_id / attempt_number / checkpoints / restart ---\n'
rg -n --hidden --glob '!**/.git/**' -S 'previous_attempt_id|attempt_number|restored_checkpoint|resume_cursor|restart|Checkpoint|execution_attempt_started' .Repository: TrogonStack/trogonai
Length of output: 249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral/static verifier: parse relevant proto/message usages and check whether
# previous_attempt_id is optional and whether any generated/validation code enforces a predecessor requirement.
python3 - <<'PY'
from pathlib import Path
import re
proto_path = Path('proto/trogonai/session/sessions/v1alpha1/execution_attempt_started.proto')
text = proto_path.read_text()
print('proto_previous_attempt_optional=', bool(re.search(r'^(?:optional|message ExecutionAttemptStarted)\s', text, re.M) and re.search(r'(?:optional\s+)?\s*string\s+previous_attempt_id\s*=\s*5\s*[;]', text, re.M)))
print('proto_attempt_number_required=', bool(re.search(r'\buint64\s+attempt_number\s*=\s*4\s*\[(?:[^\]]*)LEGACY_REQUIRED(?:[^\]]*)\]\s*;', text)))
files = [Path(p) for p in git_ls := []]
for path in git.ls_files('.'):
files.append(Path(path))
def has_call(name):
for p in files:
if p.suffix in {'.ts','.tsx','.js','.java','.go','.py','.cc','.h','.proto'}:
if re.search(r'\b'+re.escape(name)+r'\b', p.read_text(errors='ignore'), re.I):
return True
return False
terms = ['validatePreviousAttemptChain','validateAttemptNumber','previous_attempt_id','previousAttemptId','attempt_number','validateExecutionAttemptStarted','validateExecutionAttempt']
for term in terms:
print(f'matches_{term}=', bool(re.search(re.escape(term), ''.join(p.read_text(errors='ignore') for p in files), re.I)))
PYRepository: TrogonStack/trogonai
Length of output: 295
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- file exists ---'
git ls-files | rg 'proto/trogonai/session/sessions/v1alpha1/execution_attempt_started\.proto|execution_attempt_started' || true
printf '%s\n' '--- proto definition ---'
sed -n '1,160p' proto/trogonai/session/sessions/v1alpha1/execution_attempt_started.proto 2>/dev/null || true
printf '%s\n' '--- related proto files names ---'
git ls-files 'proto/trogonai/session/sessions/v1alpha1/*' | xargs -r -n1 basename | sort
printf '%s\n' '--- search previous_attempt_id / attempt_number / checkpoints / restart ---'
rg -n --hidden --glob '!**/.git/**' -S 'previous_attempt_id|attempt_number|restored_checkpoint|resume_cursor|restart|Checkpoint|execution_attempt_started' . || trueRepository: TrogonStack/trogonai
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
proto_path = Path('proto/trogonai/session/sessions/v1alpha1/execution_attempt_started.proto')
text = proto_path.read_text()
print('proto_previous_attempt_line=', next((line.strip() for line in text.splitlines() if 'previous_attempt_id' in line), '<missing>'))
is_optional_in_message = bool(re.search(r'(?:optional\s+)?\s*string\s+previous_attempt_id\s*=\s*5\s*[;]', text, re.M))
print('proto_previous_attempt_optional_like=', is_optional_in_message)
print('proto_attempt_number_required=', bool(re.search(r'\buint64\s+attempt_number\s*=\s*4\s*\[(?:[^\]]*)LEGACY_REQUIRED(?:[^\]]*)\]\s*;', text)))
all_text = ''
for path in (Path(p) for p in (p for p in __import__('subprocess').check_output(['git','ls-files'], text=True).splitlines())):
try:
all_text += path.read_text(errors='ignore')
except Exception:
pass
terms = [
'validatePreviousAttemptChain',
'validateAttemptNumber',
'previous_attempt_id',
'previousAttemptId',
'attempt_number',
'validateExecutionAttemptStarted',
'validateExecutionAttempt',
'previou',
]
for term in terms:
print(f'file_matches_{term}=', bool(re.search(term, all_text, re.I)))
PYRepository: TrogonStack/trogonai
Length of output: 623
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for path in docs/glossary/executionattempt.md ADR*.md a*/*.md; do
if [ -e "$path" ]; then
printf '%s\n' "--- $path ---"
rg -n -i "attempt|restart|previous_attempt|execution_attempt_started|session.*attempt|attempt counter|Append-only|one-active" "$path" || true
fi
done
printf '%s\n' '--- ADR-like docs containing restart/append-only attempt wording ---'
rg -n -i -S 'restart creates a new attempt|append-only|single active attempt|one-active|execution attempt|previous.*attempt|previous_attempt|attempt_number' docs . --glob '!**/.git/**' --glob '!rsworkspace/crates/platform/trogonai-proto/src/gen/**/*.rs' | head -n 200Repository: TrogonStack/trogonai
Length of output: 31133
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- ADR0035 invariant wording ---'
sed -n '410,430p' docs/adr/0035-session-store-decider-aggregate.md
sed -n '390,410p' docs/adr/0031-agent-implementation-and-session-plan.md 2>/dev/null || true
printf '%s\n' '--- focused decider validation/search ---'
rg -n -S -i --glob '!**/trogonai-proto/src/gen/**/*.rs' --glob '!**/.git/**' 'validate.*attempt|attempt.*validate|previous.*attempt|previous_attempt|attempt_number|monotonic|attempt counter|one-active|SessionStore|session store' rsworkspace crates docs --glob '!**/research/**/*.md' | head -n 250Repository: TrogonStack/trogonai
Length of output: 8032
Enforce restarted-attempt lineage in the session store.
previous_attempt_id is optional, but the session store invariant requires valid previous-attempt lineage. Reject a restarted attempt unless it references the immediately preceding attempt ID, or split this into prev_attempt_id for restarts and an explicit initial-value marker.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@proto/trogonai/session/sessions/v1alpha1/execution_attempt_started.proto`
around lines 22 - 25, Update the session-store validation for restarted attempts
using previous_attempt_id: require it to reference the immediately preceding
attempt ID, rejecting missing or nonmatching lineage; alternatively, distinguish
restart lineage with prev_attempt_id while representing initial attempts
explicitly. Preserve valid initial-attempt handling and existing attempt_number
behavior.
Comment on lines
+10
to
+11
| // Human-readable detail of the failure; empty when none. | ||
| string detail = 2 [features.field_presence = LEGACY_REQUIRED]; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -i 'session.*\.proto|session_failed\.proto|session_cancelled\.proto' . || true
echo "== git diff stat/name status =="
git diff --stat || true
git diff --name-only || true
echo "== search for detail fields in proto =="
rg -n --hidden --glob '*.proto' 'field_presence\s*=\s*LEGACY_REQUIRED|SessionCancelled|session_cancelled|session_failed|detail\s*=' . || trueRepository: TrogonStack/trogonai
Length of output: 36562
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== session_failed.proto =="
cat -n proto/trogonai/session/sessions/v1alpha1/session_failed.proto
echo
echo "== session_cancelled.proto detail context =="
sed -n '1,35p' proto/trogonai/session/sessions/v1alpha1/session_cancelled.proto | cat -n
echo
echo "== optional detail fields with "not LEGACY_REQUIRED" in session events =="
python3 - <<'PY'
from pathlib import Path
sessions = Path('proto/trogonai/session/sessions/v1alpha1')
for p in sorted(sessions.glob('*.proto')):
txt = p.read_text().splitlines()
for i,line in enumerate(txt,1):
if 'string detail' in line:
opt = 'not LEGACY_REQUIRED' in line
print(f'{p}:{i}: {line.strip()}' + (' OPTIONAL' if opt else ' REQUIRED?'))
PY
echo
echo "== proto occurrences of "LEGACY_REQUIRED detail"; search all proto files =="
rg -n --hidden --glob '*.proto' 'string detail\s*=[^;]*LEGACY_REQUIRED|LEGACY_REQUIRED;[[:space:]]*$' . | sed -n '1,120p'Repository: TrogonStack/trogonai
Length of output: 4523
🌐 Web query:
Protobuf edition 2024 LEGACY_REQUIRED field_presence string required validation
💡 Result:
In Protocol Buffers Edition 2024, the explicit required label used in proto2 has been removed [1][2]. To achieve the functionality of a required field, you must use the features.field_presence setting at the field level, specifically set to LEGACY_REQUIRED [1][2]. When a field is configured with features.field_presence = LEGACY_REQUIRED: - The field is treated as required for parsing and serialization [1]. - Any explicitly-set value is serialized onto the wire, even if it matches the default value [1]. - This behavior is intended to mirror the legacy proto2 required semantics [1][2]. Regarding validation, the LEGACY_REQUIRED setting primarily dictates presence tracking and wire-format serialization behavior [1]. It does not introduce new automatic application-layer validation logic for string content itself beyond the legacy requirement that the field must be present during parsing [1]. If specific validation is needed for the string content (e.g., non-empty, specific format), this must still be handled in your application logic or via custom validation layers, as LEGACY_REQUIRED ensures the field exists but does not enforce constraints on the string's value [1]. To apply this in your.proto file: edition = "2024"; message MyMessage { string my_field = 1 [features.field_presence = LEGACY_REQUIRED]; } For context, the default behavior for field presence in Edition 2024 is EXPLICIT, which provides presence tracking but does not enforce the field's existence during serialization or parsing in the same way LEGACY_REQUIRED does [1].
Citations:
Do not require detail when no failure detail is valid.
detail is documented as empty when absent, but LEGACY_REQUIRED treats it as a required proto2 field and can reject no-detail SessionFailed events. Align its semantics with SessionCancelled.detail: make it optional, or document and enforce explicit empty-field emission across producers/consumers.
Proposed schema adjustment
// Human-readable detail of the failure; empty when none.
- string detail = 2 [features.field_presence = LEGACY_REQUIRED];
+ string detail = 2;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Human-readable detail of the failure; empty when none. | |
| string detail = 2 [features.field_presence = LEGACY_REQUIRED]; | |
| // Human-readable detail of the failure; empty when none. | |
| string detail = 2; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@proto/trogonai/session/sessions/v1alpha1/session_failed.proto` around lines
10 - 11, Update the detail field in SessionFailed to use optional presence
semantics, matching SessionCancelled.detail, so events without failure details
remain valid and the documented empty-when-absent behavior is preserved.
The generated session types compiled only as a private detail of an unrelated feature, so no consumer could satisfy ADR#0035's typed decode-and-validate-at-the-boundary rule against them. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
The reference linter requires every ADR mention to resolve to its record. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Generated churn drowns human-authored changes in review diffs. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
The coverage gate rejects new uncovered statements, and half the validator's rules had no test exercising them. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 579ca5c. Configure here.
| // both inclusive, delimiting the kept-tail boundary and validated as an ordered | ||
| // range (covers_from <= covers_through); the model-visible view folds from the | ||
| // newest Compacted plus every event after it, and full history below the | ||
| // boundary stays on the stream (keep-forever), so no structured replacement set |
There was a problem hiding this comment.
Compacted range semantics conflict
High Severity
Compacted.covers_from / covers_through are described as both a compaction covered range and a kept-tail boundary, while the same comments also say the model-visible view is the newest summary plus every event after it. Those readings are opposites, so implementers can wire replay and context projection to opposite ends of the range and produce corrupted transcripts.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 579ca5c. Configure here.
| oneof: "operation_outcome_recorded.outcome", | ||
| }); | ||
| } | ||
| Ok(()) |
There was a problem hiding this comment.
Nested oneof arms unchecked
Medium Severity
validate_operation_outcome_recorded and validate_artifact_recorded only require the top-level oneof to be set. Nested required evidence such as OperationSucceeded.response_digest, OperationCancelled.cancelled_by, and StoredArtifact digest/mime/storage_ref is never checked, so ADR#0035 facet 3 boundary rejection for digests and shaped identifiers does not actually run on those arms.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 579ca5c. Configure here.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
rsworkspace/crates/platform/trogonai-proto/src/session/sessions/codec/tests.rs (1)
10-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated fixtures with
validate/tests.rs.
digest(),session_ordinal(),artifact_ref(), andcheckpoint()are byte-for-byte duplicated insession/sessions/validate/tests.rs. Consider a shared#[cfg(test)]fixtures module undersession/sessions/so the two suites can't drift apart as the contracts evolve.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rsworkspace/crates/platform/trogonai-proto/src/session/sessions/codec/tests.rs` around lines 10 - 56, Extract the duplicated digest, session_ordinal, artifact_ref, and checkpoint fixtures from the codec and validate test suites into a shared #[cfg(test)] fixtures module under session/sessions/. Update both suites to import and reuse these shared helpers, preserving their existing behavior and signatures.rsworkspace/crates/platform/trogonai-proto/src/session/sessions/validate/tests.rs (1)
560-609: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo validator branches remain uncovered.
items[].statusunspecified (validate.rs Line 536) andCompacted.triggerunspecified (validate.rs Line 248) have no rejection test, while nearly every other enum branch does. Worth adding for parity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rsworkspace/crates/platform/trogonai-proto/src/session/sessions/validate/tests.rs` around lines 560 - 609, Add rejection tests in the session-event validation tests for an unspecified TodoItem status and an unspecified Compacted trigger. Construct otherwise valid TodoUpdated and Compacted events using the corresponding unspecified enum values, then assert validate_session_event returns the existing validation errors from the status and trigger branches in validate.rs.rsworkspace/crates/platform/trogonai-proto/src/session/sessions/validate.rs (1)
155-166: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDigest algorithm check is case-sensitive and unconstrained.
Only the exact lowercase
"sha256"gets a length check, so"SHA256"/"sha-256"(or any unknown algorithm) passes with an arbitrary-length value. Consider an allowlist with case-insensitive comparison so unverifiable digests can't reach storage.🛡️ Sketch
fn require_digest(digest: &v1alpha1::Digest, field: &'static str) -> Result<(), SessionEventValidationError> { if digest.algorithm.is_empty() { return Err(SessionEventValidationError::EmptyDigestAlgorithm { field }); } - if digest.algorithm == "sha256" && digest.value.len() != 32 { + if digest.algorithm.eq_ignore_ascii_case("sha256") && digest.value.len() != 32 { return Err(SessionEventValidationError::Sha256DigestWrongLength { field, actual: digest.value.len(), }); } Ok(()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rsworkspace/crates/platform/trogonai-proto/src/session/sessions/validate.rs` around lines 155 - 166, Update require_digest to normalize the algorithm for case-insensitive comparison and validate it against an explicit supported-algorithm allowlist. Apply the required SHA-256 value length check for every accepted spelling, and return the appropriate validation error for unsupported algorithms instead of allowing arbitrary values through.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@rsworkspace/crates/platform/trogonai-proto/src/session/sessions/codec/tests.rs`:
- Around line 10-56: Extract the duplicated digest, session_ordinal,
artifact_ref, and checkpoint fixtures from the codec and validate test suites
into a shared #[cfg(test)] fixtures module under session/sessions/. Update both
suites to import and reuse these shared helpers, preserving their existing
behavior and signatures.
In `@rsworkspace/crates/platform/trogonai-proto/src/session/sessions/validate.rs`:
- Around line 155-166: Update require_digest to normalize the algorithm for
case-insensitive comparison and validate it against an explicit
supported-algorithm allowlist. Apply the required SHA-256 value length check for
every accepted spelling, and return the appropriate validation error for
unsupported algorithms instead of allowing arbitrary values through.
In
`@rsworkspace/crates/platform/trogonai-proto/src/session/sessions/validate/tests.rs`:
- Around line 560-609: Add rejection tests in the session-event validation tests
for an unspecified TodoItem status and an unspecified Compacted trigger.
Construct otherwise valid TodoUpdated and Compacted events using the
corresponding unspecified enum values, then assert validate_session_event
returns the existing validation errors from the status and trigger branches in
validate.rs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 11d8083e-5374-4e00-a35b-4479080041d1
📒 Files selected for processing (11)
.gitattributesdocs/adr/0035-session-store-decider-aggregate.mdrsworkspace/crates/platform/trogonai-proto/Cargo.tomlrsworkspace/crates/platform/trogonai-proto/src/lib.rsrsworkspace/crates/platform/trogonai-proto/src/session/mod.rsrsworkspace/crates/platform/trogonai-proto/src/session/sessions/codec.rsrsworkspace/crates/platform/trogonai-proto/src/session/sessions/codec/tests.rsrsworkspace/crates/platform/trogonai-proto/src/session/sessions/mod.rsrsworkspace/crates/platform/trogonai-proto/src/session/sessions/validate.rsrsworkspace/crates/platform/trogonai-proto/src/session/sessions/validate/tests.rsrsworkspace/crates/platform/trogonai-proto/src/tests.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/adr/0035-session-store-decider-aggregate.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.