docs(#6407): adopt deterministic entity context staging - #7080
Conversation
Record a runner-owned, filtered entity snapshot that is available before the pre-script and exposed through predictable paths outside the repository clone. Signed-off-by: Barak Korren <bkorren@redhat.com> Assisted-by: Codex (gpt-5.6-sol)
PR Summary by QodoDocument deterministic filtered entity-context staging
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
|
🤖 Finished Review · ✅ Success · Started 1:08 PM UTC · Completed 1:26 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.09 |
Site previewPreview: https://36defd79-site.fullsend-ai.workers.dev Commit: |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Risk Assessment: moderate (2/5) DetailsDocs-only normative spec addition with no source or CI changes; prior score of 2 preserved — Tier 2 confirms architecture.md high-churn and multi-author merge-conflict risk flagged in prior assessment, with all other signals unchanged. Previous runRisk Assessment: moderate (2/5) DetailsDocs-only ADR addition with no source or CI changes; score elevated slightly above low by docs/architecture.md being a high-churn, multi-author file that creates merge-conflict risk. |
ReviewFindingsMedium
Low
Next steps:
Previous runReviewFindingsMedium
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Separate immutable record content from mutable thread state, move the byte-level contract into a normative v1 specification, and define cleanup outside retained run artifacts. Signed-off-by: Barak Korren <bkorren@redhat.com> Assisted-by: Codex (gpt-5.6-sol)
|
🤖 Review · Commit: |
Include immutable attribution in each conversation record and define canonical whole-conversation and per-thread views whose normal reply path preserves the existing byte prefix. Signed-off-by: Barak Korren <bkorren@redhat.com> Assisted-by: Codex (gpt-5.6-sol)
|
🤖 Review · Commit: |
Store each attributed conversation record once, name records for chronological glob concatenation, and use lightweight thread order files for thread-specific assembly. Signed-off-by: Barak Korren <bkorren@redhat.com> Assisted-by: Codex (gpt-5.6-sol)
|
🤖 Finished Review · ✅ Success · Started 11:34 AM UTC · Completed 11:53 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.22 |
| ## Compatibility | ||
|
|
||
| Consumers must reject an unsupported `schema_version` or `filter_version`; they | ||
| must ignore unknown object properties within v1. Adding an optional record kind |
There was a problem hiding this comment.
[medium] internal-consistency
The Compatibility section states consumers "must ignore unknown object properties within v1" and "Adding an optional record kind or property is compatible." However, every JSON Schema in the specification sets additionalProperties: false on every object definition. A consumer validating against these schemas would reject any document containing a property added in a later compatible v1 revision, breaking the stated forward-compatibility contract.
Suggested fix: Either (a) remove additionalProperties: false from all schema objects, or (b) change the Compatibility section to state that adding any property is a breaking change requiring v2, or (c) add explicit guidance that the schemas are producer-side contracts only.
| applies size bounds, Unicode safety normalization, secret/sensitive-data | ||
| redaction, and injection scanning in that order. `filter.status` is: | ||
|
|
||
| All attacker-controlled strings in JSON metadata pass through the same pipeline |
There was a problem hiding this comment.
[low] code-organization
In the Canonical bytes section, the paragraph about attacker-controlled strings (lines 122-123) is placed between the introductory sentence ending "filter.status is:" and the bullet list defining its values (lines 125-128), breaking the semantic connection.
Suggested fix: Move lines 122-123 to immediately after the bullet list so it follows directly from "filter.status is:".
1 participant
Summary
cat records/*.mdassembly with a stable prefix for prompt cachingFULLSEND_CONTEXT_DIR, outside the repository clone and retained run artifactsRationale
Issue #6407 identifies repeated ad-hoc forge calls as non-deterministic and token-intensive. This ADR records the architecture for collecting the handled issue or change proposal once, filtering untrusted content before agent consumption, and sharing the resulting snapshot across scripts and runtimes.
The record/state split means resolution changes do not rewrite unchanged comments. Chronological record names keep prior turns byte-identical when ordinary new replies append, allowing runtimes to reuse provider prompt caches without storing duplicate concatenated views. Current mutable state is supplied afterward.
This is the design decision for #6407; implementation and token-cost measurement remain follow-up work, so this PR does not close the issue.
Verification
make lintcheck jsonhook for all v1 JSON SchemasRelates to #6407.