Live Stream: derive Anchor-owned file artifacts - #6207
franksong2702 wants to merge 22 commits into
Conversation
|
| Filename | Overview |
|---|---|
| api/artifact_references.py | New module providing bounded artifact-reference derivation, sanitization, deduplication, owner-mismatch validation, and scene-merge logic. The bytes_written check (type(bytes_written) is int and bytes_written >= 0) is strict and correctly distinguishes falsy/null values. The dist/build exclusion is correctly root-only via _IGNORED_ROOT_PATH_PARTS. Path-traversal, symlink-escape, URL, and foreign-drive guards are thorough. JSON size budgeting uses a hand-written stack-based walk that handles circular references and nesting depth. |
| api/streaming.py | Integrates artifact derivation into the live-stream producer. The two derivation sites (tool.completed event path and on_tool_complete callback) are mutually exclusive thanks to the early-return guard at line 9353. The tool.completed path lacks tool_call_id (not available in cb_kwargs for this protocol) — this is a documented limitation similar to the on_tool path. Cancel-surviving artifact_reference events, in-memory _anchor_artifact_events tracking, and _reconcile_stream_artifacts_into_terminal_anchor_scene call-sites cover all terminal paths. |
| api/routes.py | Rewrites _handle_session_anchor_scene to use a double-lock pattern: first lock resolves workspace and message identity, then journal evidence is read lock-free, then second lock revalidates and commits. Workspace identity digest is checked for consistency between both locks (409 on drift). Journal truncation fails closed for artifacts (journal_truncated skips artifact_references in replay). The _hydrate_anchor_activity_scenes reload path validates artifact workspace authority against the current session digest before retaining stored artifacts. |
| api/run_journal.py | Adds max_bytes/max_rows bounded reading to _read_jsonl with explicit truncated flag. The readline(remaining+1) sentinel correctly distinguishes a line that exactly fits (not truncated) from one that overflows (truncated). The stat() fallback for byte-limit detection is guarded against OSError. The truncated flag propagates through read_run_events so callers can fail closed for security-sensitive operations. |
| static/assistant_turn_anchors.js | Adds _boundedAnchorArtifactEvents with deduplication and 64-event/32-KiB cap mirroring the Python budget. Replaces direct array push with _appendBoundedAnchorArtifact to enforce the cap during live stream accumulation. Reload hydration now applies applySnapshotEvent for artifact rows in addition to activity rows. |
| static/messages.js | Adds artifact_reference SSE event listener (render:false) so live artifacts update the anchor without repainting the transcript. _boundedAnchorSceneArtifacts / _boundedAnchorSceneForPersistence apply the same client-side 64-event/32-KiB cap before POSTing to /anchor. hasAnchorActivityScene check extended to treat a non-empty artifacts array as a live scene even with no activity_rows. |
| static/sessions.js | Single-line change extends hasAnchorActivityScene to consider a non-empty artifacts list even when activity_rows is empty, ensuring reload treats an artifact-only scene as a valid live snapshot. |
| tests/test_live_anchor_artifact_reference.py | Comprehensive producer and lifecycle regression tests covering: strict bytes_written validation (null/bool/float/negative), path-traversal/symlink/URL rejection, root-level dist/build exclusion vs nested build allowed, budget overflow, cancel-surviving artifact events, and JavaScript budget parity via Node subprocess. The _function_block helper relies on fixed 12-space indentation — documented risk in prior review but not blocking. |
| tests/test_anchor_scene_persistence.py | Tests anchor-scene persistence with artifact ownership, workspace-digest validation, double-lock conflict detection (409), and server-authority propagation. Covers both pre-existing authoritative records and first-settlement paths. |
Sequence Diagram
sequenceDiagram
participant Agent as Hermes Agent
participant Stream as _run_agent_streaming
participant Journal as Run Journal
participant SSE as SSE Queue
participant Browser as Browser
participant Routes as /session/anchor
participant Session as Session Store
Agent->>Stream: tool_complete (write_file/patch)
Stream->>Stream: derive_file_artifact_references()
Stream->>Stream: _anchor_artifact_reference_with_workspace()
Stream->>Stream: budget check (_anchor_artifact_reference_within_stream_budget)
Stream->>Journal: put('artifact_reference', payload)
Journal-->>Stream: event_id
Stream->>Stream: _record_anchor_artifact_reference(event_id)
Stream->>SSE: artifact_reference event
Note over Stream,Session: Terminal settlement path
Stream->>Session: _reconcile_stream_artifacts_into_terminal_anchor_scene()
Session->>Session: merge_anchor_activity_scene()
Session->>Session: "save anchor_activity_scenes[key] {owner_authority: server}"
Note over Browser,Routes: Browser settlement path
SSE-->>Browser: artifact_reference SSE event
Browser->>Browser: "_applyToAnchor('artifact_reference', {render:false})"
Browser->>Browser: _boundedAnchorArtifactEvents()
Browser->>Routes: "POST /session/{id}/anchor {scene: {artifacts: [...]}}"
Routes->>Journal: _run_journal_live_snapshot(stream_id)
Note over Routes,Journal: Read bounded journal for authority evidence
Routes->>Routes: retain_server_authoritative_artifact_events()
Routes->>Routes: merge_anchor_activity_scene()
Routes->>Session: "save anchor_activity_scenes[ref] {artifact_authority: server}"
Note over Browser,Session: Reload hydration
Browser->>Routes: "GET /session/{id}"
Routes->>Session: _hydrate_anchor_activity_scenes()
Routes->>Routes: validate workspace_id digest match
Routes->>Routes: retain_server_authoritative_artifact_events()
Routes-->>Browser: messages with _anchor_activity_scene.artifacts
Reviews (84): Last reviewed commit: "chore: refresh PR 6207 onto master" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Aftercare follow-up pushed in b5f4f77:
Local verification for the follow-up:
|
|
Base refresh pushed in e1cf64a:
GitHub Actions and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in 3c4842e:
GitHub checks have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in c6dffcb:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in 180dc6a:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Proof-matrix aftercare at |
|
Base refresh pushed in 388575f:
GitHub checks have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in a86ce5b:
GitHub checks have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in 009e517:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in 7eaf437:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in 90b8513:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in c173e27:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in 3911bd3:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Reviewed at head Verified fail-closed derivation, path-only emission, and traversal/symlink containment against the real tool-result contract; ran the suite (8/8 pass) and confirmed coverage is genuine by breaking the fail-closed check and watching
Two optional notes, neither blocking: (1) |
|
Base refresh pushed in d7d01a4:
Thanks for the clean review on 3911bd3. I’m leaving the two optional notes as non-blocking for this PR:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in ebfc1b4:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in be9986d:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in 942f930:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in f0293d7:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in
Verification:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in
Verification:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
|
Base refresh pushed in
Verification:
GitHub checks and Greptile have restarted on the new head; no Frank action is needed. |
c32a578 to
2064140
Compare
|
Rebuilt this branch from current
The PR body now reflects the rebuilt scope and current verification. |
nesquena-hermes
left a comment
nesquena-hermes
left a comment
There was a problem hiding this comment.
Deep warm-up re-gate: changes requested
Certified head: 206414025777caa736cccc32c0fec098dc3fa0fb
The force-rebuild successfully removes the unrelated model/profile/config/atomic-write bundle, and the current artifact pipeline has strong path, owner, and budget checks. One deterministic authority gap remains in the core feature.
Blocking: a browser scene can invent an artifact that no file tool produced
_handle_session_anchor_scene() proves only that the parent assistant message has a trusted run/stream identity. It then merges the raw browser scene.artifacts list and marks the resulting record owner_authority="server". A browser artifact that repeats the trusted session/run/stream IDs therefore survives even when there is no matching server-derived artifact event, tool result, or file mutation.
I reproduced the full current-head path in the mandatory sandbox:
- settle a cancelled turn through the real server finalizer with
artifact_events=[]; - POST a matching-owner client scene containing canonical-looking
reports/never-written.md; - save and reload the session;
- hydrate through
_hydrate_anchor_activity_scenes().
The route returned 200, persisted the invented descriptor, and hydrated it again although the file did not exist and no canonical write_file/patch result had produced it. This does not need a traversal or owner-ID bypass; canonical path syntax is being mistaken for mutation authority.
Required fix
- Treat browser
scene.artifactsas an untrusted projection. Retain an incoming row only when it exactly matches a server-derived artifact descriptor/event for that session/run/stream, including canonical path, source tool,tool_call_id, and preferably authoritative event ID/sequence. If the canonical server set is empty, reject or drop every incoming artifact. - Do not promote a client-created artifact set to server authority merely because the parent message has trusted run/stream IDs. Preserve artifact authority only from server reconciliation/journal evidence.
- Validate the merged scene, and fail closed during read-side hydration for legacy/corrupt persisted artifact rows.
- Add a production-composed cancellation regression with zero canonical artifacts, a matching-owner invented browser row, save/reload, and hydration. Add controls proving an exact server-reconciled row survives once while wrong
source_tool/tool_call_idvariants cannot supplement or replace it.
Verification
- Layer-1 threat scan: CLEAN
- Mandatory no-network/no-secrets sandbox:
test_live_anchor_artifact_reference.py25 passed;test_anchor_scene_persistence.py49 passed - Reviewer production-path diagnostic: 1 passed, confirming the unsafe accept → persist → reload behavior above
- Exact head remained clean and unchanged after diagnostic cleanup
The visible artifact presentation still deserves the existing product/design sign-off, but this landed-evidence bypass is an objective data-authority defect and needs code/test rework first. No merge, approval, release, contributor-branch write, screenshot, or production action was performed.
nesquena-hermes
left a comment
nesquena-hermes
left a comment
There was a problem hiding this comment.
Deep warm-up re-gate: changes still requested
Certified head: 09d406e957009ba471c0dc182d3d4552f53fff0d
Thanks for the substantial response. The fresh browser-mint path from the prior review is now fixed: _handle_session_anchor_scene() substitutes only exact server journal/existing evidence, and the new authority key binds event ID, owner IDs, sequence, kind, path, source tool, and tool-call ID. The submitted accept → persist → hydrate regression now passes.
One deterministic lifecycle hole remains in the required legacy/corrupt-row fail-closed boundary.
Blocking: terminal reconciliation promotes an untrusted legacy artifact set to server authority
api/streaming.py::_reconcile_stream_artifacts_into_terminal_anchor_scene() reads the whole pre-existing scene at lines 2125-2127, merges every existing artifact with the current server-derived artifact at 2150-2164, then stamps the entire merged record artifact_authority="server" at 2168-2176. It never checks whether the existing record already had server artifact authority.
I reproduced the exact current-head path in the mandatory sandbox:
- seed the persisted pre-fix shape: matching server-owned message/scene, no
artifact_authority, and browser-inventedreports/never-written.md; - deliver one legitimate server-derived
patchevent forreports/really-written.md; - call the real terminal reconciler, save, reload, and run production hydration.
Both paths survived and rehydrated under the new server marker. One genuine mutation therefore launders an unrelated unknown browser artifact that has no durable file-tool evidence. This violates item 3 of the prior fix-spec: legacy/corrupt persisted artifact rows must fail closed.
Required fix
- Before
merge_anchor_activity_scene(), sanitize the existing artifact set. Ifrecord["artifact_authority"] != "server", deep-copy the existing scene and clear its artifacts. If it is server-authoritative, retain only canonical owner/path-valid rows. - Merge that filtered scene with the current call's server-derived artifacts. Set
artifact_authority="server"only for the resulting server-evidenced set, never as a blanket promotion of an unmarked legacy scene. - Add a production-composed regression: an unmarked pre-fix matching-owner record with an invented artifact plus one legitimate late
patch/write_fileevent → real terminal reconcile → save/reload/hydrate. Assert only the legitimate artifact survives once, with exact event/tool/sequence/tool-call identity, and the invented row remains absent.
Gate evidence
- Layer-1 threat scan: CLEAN.
- Mandatory sandbox submitted targets: 102 passed / 0 failed.
- Reviewer production-path legacy-laundering diagnostic: 1 passed, reproducing the unsafe promotion.
- Positive real-journal control: 1 passed, proving legitimate server evidence still survives cold-cache-like lookup, POST, persistence, reload, and hydration.
- Final post-diagnostic sandbox rerun: 102 passed / 0 failed; reviewer probes deleted; worktree clean.
The existing artifact-presentation design qualifier remains after this authority defect is fixed.
nesquena-hermes
added
changes-requested
nesquena-hermes
left a comment
nesquena-hermes
left a comment
There was a problem hiding this comment.
Watch re-gate: changes still requested
Certified head: dcaa34d23c5d4c3c840f45b82898aae7388faf04
Thanks for the follow-up. The new clear/filter branch is directionally correct, but the submitted legacy lifecycle regression never sends its invented row through that branch. The prior fail-closed acceptance item therefore is not yet proved, and the stale invented row remains durably persisted.
Blocking: the regression is false-green because it writes a different record key
_reconcile_stream_artifacts_into_terminal_anchor_scene() computes the assistant's SHA-256 message ref and reads only records.get(key) (api/streaming.py:2125-2129). The new test seeds the unmarked legacy scene under literal key index:1 (tests/test_live_anchor_artifact_reference.py:1323-1333). For this assistant message the digest is non-empty, so reconciliation reads an empty record, creates a second digest-keyed server record, and leaves the original index:1 row unchanged.
Session.save() serializes both records. On reload, _hydrate_anchor_activity_scenes() builds by_index with last-write-wins assignment (api/routes.py:4929-4943); the later digest-keyed record shadows the stale index:1 record. The test checks only the hydrated winner, not the durable record map. It therefore passes even though the invented legacy artifact is still saved.
The marked-record filter also calls retain_server_authoritative_artifact_events([raw_artifact], [raw_artifact], ...), using each persisted row as both authority and projection. That is self-authentication rather than reconciliation against independent server evidence. At minimum, marked rows need explicit, internally consistent session/run/stream/event/sequence identity before they are retained.
Required fix
- Resolve the target record by canonical message ref and, when it is absent, by one unambiguous legacy record with the same
message_indexand matching session/run/stream ownership. Remove the legacy key when writing the canonical record so only one durable record remains. - Deep-copy that record, clear artifacts when
artifact_authority != "server", and strictly validate any retained marked row's explicit owner fields, event/sequence identity, and canonical workspace path before merging the current server-derived events. - Strengthen the production-composed regression with both canonical-keyed and
index:1legacy cases. After reconcile → save → reload, assert the durable record map contains exactly one canonical record, the invented path/event ID is absent from every persisted record, and the sole durable and hydrated artifact exactly matches the real event's session/run/stream/event/seq/path/source-tool/tool-call tuple. The test should fail if the clear/migration branch is removed.
Gate evidence
- Layer-1 threat scan: CLEAN, score 0.
- Mandatory no-network/no-secrets sandbox: focused terminal/legacy/authority slice 5 passed, 21 deselected; exact new test 1 passed; neighboring persistence-authority slice 3 passed, 48 deselected.
- Those green results expose the oracle gap above; no full suite or screenshot was run in this watcher pass.
This is the same required legacy/corrupt-row fail-closed boundary from the prior review, so the PR remains in contributor court. No merge, approval, release, contributor-branch write, screenshot, or production action was performed.
|
Addressed the watch re-gate from The three requested boundaries are now covered by
Hosted validation on this exact head is fully green: 23/23 checks, including browser smoke, docs, lint, all lifecycle scenarios, and the Python 3.11/3.12/3.13 matrix. Requesting a fresh exact-head re-gate. |
Gate certification: FAILPR: #6207 — Evidence
Blocking finding: artifact authority survives workspace rebindingServer artifact evidence is not bound to the workspace in which the mutation occurred:
Reproduction on the exact head:
The fail-closed probe expected no artifacts after rebinding, but received the original server-authoritative event while the two files contained Required fix
Additional blocking finding: artifact verification is not input/scan boundedThe output list is bounded, but the authority pipeline still performs work over the complete raw input before and after normalization:
Required remediation:
Result: This certification does not merge, deploy, tag, close, or otherwise release the PR. |
|
Addressed the 2026-08-05 gate findings on exact head Workspace authority
Bounded verification
Local verification:
Hosted checks are running. Requesting a fresh exact-head warm-up and full re-gate after they settle. |
|
Refreshed PR #6207 onto current origin/master and resolved the streaming integration conflicts on exact head 7a74e09. The resolution preserves both sides of the required invariant: current-session/writeback-owner/active-stream generation guards run before cancellation persistence, while the PR artifact helper and server-authoritative artifact reconciliation retain run_id and artifact_events. One integration regression in the Anchor persistence test was corrected to establish the master writeback-owner/current-session precondition. Coordinator verification:
Full local suite was not entirely green: 14,197 passed, 130 skipped, 1 xfailed, 2 xpassed, 8 failed. On targeted rerun, MiniMax passed; one macOS setgid permission assertion and six OpenAI TTS environment failures persisted. The affected implementations and tests are byte-identical to origin/master, so I did not mix unrelated fixes into this PR. Conflict analysis assistance: Luna Max worker. I performed the merge resolution, diff review, regression correction, verification, commit, and push. Please re-review exact head 7a74e09 when the external check settles. |
2 participants
Thinking Path
artifact_reference, but successful Hermes file tools did not produce a durable, turn-owned artifact event.What Changed
write_fileandpatchresults.artifact_referenceevents aftertool_completeand retain the exact run/stream owner through the run journal.Fixes #6205.
Refs #3400.
Why It Matters
Files produced by a turn remain attached to that same assistant turn even when Live Stream transitions to Final Answer without a connected browser. The backend records only bounded workspace-relative metadata; it never persists file bodies or raw tool results in the Anchor scene.
Release note: Preserve turn-owned file artifact references from successful file tools across streaming, terminal settlement, reconnect, and reload.
Verification
./scripts/test.sh tests/test_live_anchor_artifact_reference.py tests/test_anchor_scene_persistence.py tests/test_sse_relay_apperror_closes.py --tb=short -q-> 79 passed.origin/master@dd7f6ac318in the same environment.python scripts/ruff_lint.py --diff origin/master-> 0 findings on added/modified lines.npm run --silent lint:runtime-> passed.git diff --check origin/master-> passed.Risks / Follow-ups
write_fileandpatchresult schemas are recognized. Shell, code, and dynamic MCP side effects remain out of scope because their mutations cannot be proven from command text.Model Used
gpt-5.6-solimplemented and self-reviewed the rebuild.gpt-5.6-lunaperformed an independent read-only scope and correctness review.Contract Routing
Task type: implementing the accepted Live-to-Final artifact-ownership contract.
Touched areas:
Relevant docs:
AGENTS.mdCONTRIBUTING.mddocs/CONTRACTS.mddocs/rfcs/live-to-final-assistant-replies.mddocs/rfcs/stable-assistant-turn-anchors.mddocs/architecture/stable-assistant-turn-anchor-phase0.mdState invariant: proven tool mutation -> bounded workspace-relative
artifact_reference-> run journal -> exact Assistant Turn owner -> renderer-neutralactivity_scene_v1, without becoming a Compact Worklog row.Contract Change
Previous state: the Anchor model classified artifact references, but production file mutations did not emit or durably settle them.
New state: successful canonical file mutations produce bounded, server-validated artifact references that survive all terminal and reload paths.
Compatibility: additive metadata only. Existing tool cards, final-answer rendering, and Artifact-tab behavior remain unchanged.