feat(gui): add electron step log archiver for the marker protocol - #178
Draft
Emin017 wants to merge 27 commits into
Draft
feat(gui): add electron step log archiver for the marker protocol#178Emin017 wants to merge 27 commits into
Emin017 wants to merge 27 commits into
Conversation
Signed-off-by: Emin <me@emin.chat>
The sidecar never writes step log files; it emits bytes and step markers on stderr. The new StepLogArchiver consumes that stream in the Electron process: a marker state machine ported from the normative Python reader, per-step archives with truncate-on-begin, flow.json allowlist, name sanitization plus symlink-aware workspace containment, batched segments with byte-accurate cursors, a bounded per-step tail for finalLog, and unscoped-byte routing. Disallowed or unsafe markers degrade to ordinary bytes.
…chiver Wire the sidecar's stderr through the step log archiver and add the event bridge that owns operation context and crash-safe ordering: - step.log events are synthesized with the active operation's identity (kind, origin, rerun, operationId, runSessionId, runtimeInstanceId, workspaceId), a minted eventId, and the most recently observed ecc sequence; ordering within a step is carried by payload.cursor. - Segments for a step are buffered until its step.started has passed the tracker and fanout; buffer overflow drops only synthesis while the archive file stays complete. - step.completed is held until the archiver's matching StepEnded, with a bounded timeout and sidecar close as cancellation sources; release flushes pending bytes and attaches the bounded finalLog tail. - Unscoped bytes keep today's behavior exactly (sidecar log file plus runtime.stderr); scoped bytes no longer appear in either. - The flow.json allowlist refreshes on workspace open, operation start, and rerun preparation. Synthesized events bypass the operation tracker by construction.
Electron-side integration tests drive notifications plus raw sidecar stderr through the wired runtime: synthesized step.log events fan out with workspace context and never touch the operation tracker, unscoped bytes keep the sidecar log plus runtime.stderr path with no marker leakage, and a held step.completed releases with finalLog before runtime.exited when the sidecar dies mid-step. Renderer-side tests pin the synthesized shape against the live event mapping: a full-flow synthesized step.log still maps to cmd rtl2gds, and repeated sequence values are accepted on synthesized events.
Review follow-ups for the electron step log synthesis: - Segments are now buffered whenever the operation context or the step's step.started has not arrived yet, so stderr that races ahead of the RPC channel is released with the operation identity that actually produced it; stale buffers drop at terminal events and sidecar close instead of being misattributed. - Skipped steps never emit markers, so their step.completed is forwarded immediately instead of waiting out the hold timeout and reordering the lifecycle; a superseded hold now logs a note when it releases. - The archiver close path routes parser-held bytes (a truncated final line or marker candidate) into the archive or the unscoped stream instead of discarding them. - Synthesized events track the most recently observed sequence rather than the maximum, and all lifecycle state resets on sidecar close. - The bridge construction and protocol-event shaping move into runtimeProtocolFanout.ts, keeping workspaceRuntime.ts under 700 lines.
Pin the ecc submodule to the branch that removes executor-side step log writing and server-side tailing (marker protocol v1, end-marker ordering, CLI worker unification, operations.py tail deletion). This tree pairs the marker-producing ecc with the archiver-equipped GUI, so checking out this PR always yields a consistent producer/consumer pair; bumping ecc without the GUI archiver would break GUI step logs.
- The archiver keeps pending bytes and per-step tails as chunk lists with byte counters, replacing per-line whole-buffer concats with one concat per flush or tail read. - Workspace containment reuses electron/services/pathScope's isPathWithinRoot instead of a local copy. - The allowlist has a single key-based refresh entry point. - Bump the ecc pin for the runtime/cli reuse cleanup.
Port the arbitrary-boundary scanning rule to the Electron archiver: bytes before a candidate frame are ordinary data, trailing partial prefixes are held back, incomplete candidates stay bounded, and only valid newline-terminated v1 frames are consumed. Covers markers glued to unterminated output, frames split across chunks, mid-line invalid frames, and recovery after an overlong candidate.
A failed step does not wait for the render gate, so step.completed and operation.failed can both reach the RPC channel before the stderr end marker. The bridge now queues terminal events behind a held completion and forwards them in lifecycle order once the hold releases. Hold timeouts and superseded holds abandon the archiver's stale active step, so the next begin starts a fresh attempt instead of reading as nested, and every forwarded completion now consumes its attempt count regardless of how it was released.
An activation-time open failure no longer installs step state: the begin frame and the step's bytes route to the unscoped path (sidecar log plus runtime.stderr) with a logged violation, so the output stays visible. Tail-only abandonment remains reserved for mid-step write failures. The in-memory tail now also slices oversized single chunks on append, keeping its byte bound strict.
The synthesized-event mapping coverage (rtl2gds command mapping, repeated-sequence tolerance) moves into its own file so the existing contract suite remains an untouched regression net for AC-7.
ecc only admits a new operation after the previous one went terminal, so a pending end marker can no longer arrive: the bridge now releases the previous operation's held step.completed and forwards its queued terminal event before the new operation.started, preserving lifecycle order across the boundary instead of letting the terminal event overtake the held completion.
Re-pin for the round-1 ecc fixes: arbitrary-boundary marker scanning and RPC-error unmatched-step repair.
When the executor raises after begin, neither an end marker nor a step.completed exists. The bridge now holds a terminal operation event behind the archiver's StepEnded while an archive is still active: on a matching end the terminal forwards immediately; on the bounded timeout, sidecar close, or a superseding operation.started the stale archive is flushed and abandoned first, so the next operation's begin starts a fresh attempt and no late segment inherits its identity.
The operation tracker used to resolve terminal waiters before the step log bridge's barrier released, so a new operation could start while the stale archive was still active. Tracker, terminal lifecycle, and fanout now live in one trackAndEmitProtocolEvent callback that runs when the bridge forwards: synthesized step.log events stay emit-only, step.started is tracked before buffered segments release, and a held terminal keeps waitForOperation pending until StepEnded, timeout, or close. Render-gate replays of step.completed forward immediately instead of being re-held.
Move the shutdownBarrierFrom parser into runtimeClient.ts, which owns the RPC shutdown result boundary, returning workspaceRuntime.ts to 691 lines with the tracker/barrier ownership unchanged. Split the bridge test surface: shared harness and event factories live in stepLogEventBridge.testSupport.ts, and the terminal/crash/supersession/ replay lifecycle cases move to stepLogEventBridge.lifecycle.test.ts — every resulting module is under 700 lines with all assertions preserved, plus a new guard test for render-gate replayed completions.
The holdback bound is now inclusive in the Electron archiver, matching the specification and the Python reader: an incomplete candidate of exactly 512 bytes is held until its newline arrives, while anything longer degrades to ordinary bytes. Boundary tests cover both sides.
The Python reader rejects a marker whose payload is not valid UTF-8 (UnicodeDecodeError), but the TS archiver decoded lossy first, so an invalid byte could still parse as a valid JSON marker. Switch parseStepMarker to a fatal TextDecoder so a non-UTF-8 payload is ordinary stream bytes on both consumers, and pin the parity with a normative test.
Bumps the submodule past the two round-5 review fixes: the normative non-UTF-8 marker rejection test and the executable-steps chaining regression test.
The bridge cleared endedStepCounts when operation.started arrived, so a StepEnded that raced ahead on stderr was erased and the later step.completed waited out the full hold timeout. Replace the cumulative ended/completed totals with per-key pending-end tokens: a matched StepEnded adds one, a forwarded completion consumes one, and the remaining tokens are cleared only when the previous operation's terminal is actually forwarded (including timeout, close, and supersession paths). The pre-operation race test now runs through step.completed and asserts immediate forwarding with the complete finalLog and no timer; a lifecycle regression proves a crashed operation's leftover token cannot satisfy the next operation's completion.
The operation.started supersession path for a held terminal cleared only bufferedSegments, so a leftover pending-end token from the crashed operation could satisfy the next operation's same-key completion before its own end marker. Route the release through clearCorrelationState() like every other terminal-forward path, with a regression covering heldTerminal supersession followed by a same-key completion.
# Conflicts: # ecc # ecos/gui/apps/desktop-electron/electron/services/eccRpc/sidecarProcess.ts
Batch flushes (size threshold or window timer) can split a multibyte UTF-8 code point; decoding each flush independently emitted replacement glyphs into synthesized step.log text even though the archived bytes were correct. Scoped segments now decode through a streaming TextDecoder reset on each step activation, and unscoped text through one finalized on close, so split characters reconstruct exactly. Regressions pin both boundary cases with the archived bytes intact.
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
1 participant
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.
Summary
Scope
Select the areas touched by this PR:
ecos/gui, including renderer, Electron, and shared packages.Validation
List the commands you ran. Mark checks that are not applicable as N/A.
cd ecos/gui && pnpm run typecheckcd ecos/gui && pnpm run testcd ecos/gui && pnpm run buildmake buildmake demo-gcdmake demo-retrosoccd ecos/gui && pnpm run devSkipped checks and reason:
Screenshots or Recordings
Required for visible GUI changes.
Release, Packaging, and Runtime Impact
Notes:
Checklist