Skip to content

fix(sessions): exclude workflow stage sessions from resume history - #1510

Merged
flora131 merged 3 commits into
mainfrom
issue-1504-impl
Jun 25, 2026
Merged

fix(sessions): exclude workflow stage sessions from resume history#1510
flora131 merged 3 commits into
mainfrom
issue-1504-impl

Conversation

@flora131

@flora131 flora131 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Workflow stage sessions are now marked as internal in their SessionHeader and excluded from the standard /resume, atomic -r, and --continue history by default — keeping the resume picker focused on interactive sessions while workflow stages remain fully accessible through workflow-specific commands and direct file access.

Key Changes

  • SessionWorkflowMetadata type (session-manager-types.ts) — captures runId/stageId/stageName linkage on internal session headers
  • SessionHeader / SessionInfo extension — new optional internal: true and workflow fields propagate through the full session lifecycle
  • markSessionInternal(workflow?) method on SessionManager — stamps a session as internal post-creation; idempotent (preserves existing full marker on reattach)
  • SDK auto-marking (sdk.ts) — createAgentSession stamps the internal marker automatically when orchestrationContext.kind === "workflow-stage"
  • Header prefilter optimization (session-manager-list.ts) — listing reads only the lightweight header before skipping internal sessions, avoiding the expensive full 1 MiB transcript parse for hidden sessions
  • Robust readSessionHeader (session-manager-storage.ts) — replaces the old fixed 512-byte window with a proper chunked line-reader using a dedicated 64 KB buffer; correctly handles headers larger than one chunk (e.g. workflow sessions carrying long stage metadata) and avoids decoder flush corruption when a newline is found mid-buffer
  • includeInternal opt-inSessionManager.list, listAll, and continueRecent accept { includeInternal: true } for workflow-specific resume paths and diagnostics
  • Docs — updated docs/session-format.md, docs/sessions.md, docs/workflows.md and both package changelogs
  • Regression tests (test/session-manager/internal-sessions.test.ts, sdk-session-manager.test.ts) — covers filtering, workflow metadata visibility, robust multi-chunk header reads, decoder-flush correctness, prefilter skipping of malformed internal sessions, and SDK-level marking

Migration Notes

Legacy workflow sessions created before this change lack the internal marker and will continue to appear in the standard history until they age out or are deleted. No action required.

Validation

bun test packages/coding-agent/test/session-manager/ packages/coding-agent/test/sdk-session-manager.test.ts
bun test packages/coding-agent/test/first-run-onboarding*.test.ts
bun run typecheck
bun run lint
bun run check:file-length

Fixes #1504

flora131 added 3 commits June 25, 2026 08:23
Mark workflow-stage sessions as internal and filter them from standard resume/list/continue paths by default, while preserving includeInternal opt-ins for workflow resume and direct session access.

Refs: #1504
Assistant-model: OpenAI ChatGPT
…essions

- readSessionHeader no longer calls decoder.end() after a newline is
  found, preventing extra bytes from data after the header line from
  corrupting the parsed header (affects >1MiB headers spanning multiple
  read chunks).
- listSessionsFromDir and listAllSessions now prefilter internal sessions
  via readSessionHeader before the expensive buildSessionInfo transcript
  parse, preserving includeInternal opt-in behavior.
- Added regression tests for >1MiB header newline handling, single-line
  files, and malformed-body internal sessions.

Refs: #1504
readSessionHeader now reads in 64KB chunks via a dedicated
HEADER_READ_BUFFER_SIZE instead of the 1MiB transcript buffer.
This keeps the early internal-session prefilter cheap while still
reading the full first line across chunks. Decoder.end() remains
guarded to only flush on EOF without a newline.

Refs: #1504
@flora131

Copy link
Copy Markdown
Collaborator Author

Implementation Notes

Task: Implement GitHub issue #1504 in bastani-inc/atomic: #1504 ("Exclude workflow sessions from standard resume history").

Start by reading the full issue body with gh issue view 1504 --repo bastani-inc/atomic and inspect the relevant code/tests. Scope the work tightly to the issue acceptance criteria:

  • Mark sessions created by workflow execution as internal/workflow sessions in metadata.
  • Exclude workflow-created sessions from the standard /resume history.
  • Keep workflow sessions discoverable/resumable through /workflow resume or the workflow-specific resume path.
  • Add regression tests covering standard resume filtering and workflow resume visibility.
  • Update user-facing docs in packages/coding-agent/docs and the relevant package CHANGELOG under Unreleased if behavior changes are user-visible.

Repo rules: use Bun commands only (bun, bun run, bun test), keep authored source files <=500 lines, avoid any/unknown, and do not bump versions. Validate with targeted tests plus typecheck/lint where practical. Commit the implementation and create a pull request when done.

Running Notes

  • Record implementation decisions, deviations from the research findings, tradeoffs, blockers, validation notes, and anything else the user should know.

Decisions / tradeoffs

  • Header-based marker (not custom entries). Followed the analysis recommendation: added backward-compatible optional SessionHeader.internal (boolean) and SessionHeader.workflow ({ runId, stageId, stageName }) fields. This makes filtering cheap because the standard resume paths (findMostRecentSession, readSessionHeader, buildSessionInfo) only need to read the first JSONL line — no need to parse the whole file.
  • NewSessionOptions threading + markSessionInternal. NewSessionOptions now carries internal/workflow so SessionManager.create({...}) stamps the marker at construction. Because workflow stages sometimes supply an externally-built SessionManager, I also added SessionManager.markSessionInternal(workflow?), which mutates the in-memory header (and rewrites the file if already flushed) while preserving an existing full marker on reattach.
  • SDK translation of orchestration context. In sdk.ts (createAgentSession), when options.orchestrationContext?.kind === "workflow-stage", the SDK calls sessionManager.markSessionInternal({ runId, stageId, stageName }). This covers both the SDK-created SessionManager and externally-supplied ones, and reattach preserves existing linkage. No change was needed in packages/workflows/src/extension/wiring.ts because the workflow runtime already passes the workflow-stage orchestration context via withWorkflowStageSessionOptions.
  • Filtering defaults. SessionManager.list/listAll/continueRecent, listProjectSessions/listAllSessions/listSessionsFromDir, and findMostRecentSession all exclude internal sessions by default. Each gained an includeInternal opt-in (default false) for the workflow resume path, debugging, and tests.
  • readSessionHeader robustness. Replaced the fixed 512-byte read with a full-first-line read (buffered in 1MB chunks, stopped at the first \n) so very long internal/workflow headers are not truncated/dropped from listing and resume filtering. This was an explicit ask in the analysis tradeoffs.
  • Barrel exports. Exported SessionWorkflowMetadata type and the isInternalHeader helper from the session-manager.ts barrel.

Tradeoffs made explicit

  • Legacy workflow sessions created before this feature have no persisted internal marker and will remain visible in the standard /resume history until they age out or are deleted. There is no reliable way to retroactively classify them. Documented in sessions.md and workflows.md.
  • Direct/explicit access is unchanged. --session <path> and /workflow resume/workflow({ action: "resume" }) still open workflow stage sessions directly; only the standard history is filtered.
  • User-initiated forks of workflow sessions are not auto-marked internal; only sessions actually created under a workflow-stage orchestration context inherit the marker.

Validation

  • bun run typecheck — clean.
  • bun run lint (tsc --noEmit) — clean.
  • bun run check:file-length — passes (session-manager-core.ts is exactly 500 lines; trimmed JSDoc to stay under the gate rather than splitting the class).
  • bun run test test/session-manager test/sdk-session-manager.test.ts — 111/111 pass (11 new internal-session tests + 1 new SDK workflow-stage marking test).
  • bun run test (full coding-agent suite) — the only failures are pre-existing and unrelated (bash-pty-native, hashline-tools, search-tool-compatibility/parity-edges); verified identical failures on a clean git stash checkout.
  • bun run test (full workflows suite) — 2528 pass, 21 pre-existing unrelated failures (version metadata, cursor registration, release workflows, builtin-resource discovery); verified identical count on a clean git stash checkout. All executor/wiring/stage-session/resume tests pass.

Files changed

  • packages/coding-agent/src/core/session-manager-types.tsSessionWorkflowMetadata, SessionHeader.internal/workflow, NewSessionOptions.internal/workflow, SessionInfo.internal/workflow.
  • packages/coding-agent/src/core/session-manager-entries.tscreateSessionHeader accepts internal/workflow.
  • packages/coding-agent/src/core/session-manager-core.tsnewSession threads metadata, markSessionInternal method, list/listAll/continueRecent includeInternal options.
  • packages/coding-agent/src/core/session-manager-list.tsbuildSessionInfo extracts internal/workflow; list functions accept/filter includeInternal.
  • packages/coding-agent/src/core/session-manager-storage.ts — robust readSessionHeader, isInternalHeader, findMostRecentSession includeInternal.
  • packages/coding-agent/src/core/session-manager.ts — barrel exports.
  • packages/coding-agent/src/core/sdk.ts — mark workflow-stage sessions internal.
  • packages/coding-agent/test/session-manager/internal-sessions.test.ts — new regression tests (11 tests).
  • packages/coding-agent/test/sdk-session-manager.test.ts — new SDK workflow-stage marking test.
  • packages/coding-agent/docs/session-format.md, sessions.md, workflows.md — docs.
  • packages/coding-agent/CHANGELOG.md, packages/workflows/CHANGELOG.md — Unreleased entries.

Commit

  • Created commit 2a76c5eda0c77f7fbd788a689d5205c1d819d45f (fix: exclude workflow sessions from resume history, Refs: #1504).
  • No pull request was created in this stage per orchestration instructions.
  • Left research artifacts under research/docs/2026-06-25-issue-1504-*.md uncommitted because they were not part of the implementation changes.

QA E2E Video

No QA E2E video applies. This change is a backend session-history filtering behavior (TUI/CLI resume history) with no user-visible web/UI scenario that playwright-cli can drive in this checkout; correctness is covered by the unit + SDK regression tests above.

Follow-up: reviewer robustness fixes (commit 9205d7449)

Applied two reviewer findings from the latest research:

  1. readSessionHeader decoder flush corruption. Previously, after finding a newline and breaking the read loop, decoder.end() was still called. This flushed leftover decoder bytes (belonging to data after the header line) into the parsed header string, corrupting valid headers — particularly >1MiB headers that span multiple 1MiB read chunks. Fix: track foundNewline and only call decoder.end() when EOF is reached without a newline. Added regression tests for >1MiB headers with a trailing body line and for single-line files with no newline.

  2. Header prefiltering in session-manager-list.ts. listSessionsFromDir and listAllSessions now call readSessionHeader + isInternalHeader before buildSessionInfo, skipping hidden internal sessions without the expensive full-transcript parse. includeInternal opt-in is preserved. Added regression tests demonstrating an internal session with a malformed body is skipped by default (no throw) and surfaced with includeInternal.

Test discovery note: the malformed-body test revealed that parseSessionEntries tolerates individual unparseable lines, so a session with a bad body line is still returned (with 0 messages) under includeInternal rather than being dropped — the key guarantee is that it does not throw.

Validation (follow-up):

  • bun test packages/coding-agent/test/session-manager/ packages/coding-agent/test/sdk-session-manager.test.ts — 115/115 pass (4 new tests added).
  • bun run typecheck — clean.
  • bun run lint — clean.
  • bun run check:file-length — passes.
  • Pre-commit hooks (lint, file-length, test:unit) all passed on commit.

Docs/changelog: No changes needed for these internal robustness fixes; they do not introduce new user-visible behavior beyond what was already documented.

Follow-up: origin/main integration and small header buffer (commits 61066df, f0521b5, 95549fc)

Resolved the latest research findings (P2 and P3):

  1. P2 — stale raw tree vs origin/main. The branch was mid-rebase onto origin/main (d9b1d0ca3, which includes the onboarding feat(onboarding): first-run CTA and workflow-routing handoff #1488 merge). Completed the rebase: both Exclude workflow sessions from standard resume history #1504 commits now sit cleanly on top of the onboarding commit. Confirmed onboarding code (packages/coding-agent/src/modes/interactive/interactive-onboarding.ts) and onboarding tests are preserved and pass (33/33 across three first-run-onboarding suites). New commit hashes after rebase: 61066df81 and f0521b54b.

  2. P3 — small dedicated header buffer. readSessionHeader() previously reused the 1MiB SESSION_READ_BUFFER_SIZE transcript buffer. Added a dedicated HEADER_READ_BUFFER_SIZE = 64 * 1024 used only by readSessionHeader, while loadEntriesFromFile still uses the 1MiB buffer for full transcript parsing. The read loop still reads in chunks until the first newline (or EOF), so headers larger than one chunk are handled correctly, and decoder.end() remains guarded to only flush on EOF without a newline. This keeps the early internal-session prefilter in session-manager-list.ts cheap. Added a regression test for a header larger than the 64KB chunk but smaller than 1MiB, proving cross-chunk accumulation with the small buffer. Commit 95549fcb8.

Validation:

  • bun test packages/coding-agent/test/session-manager/ packages/coding-agent/test/sdk-session-manager.test.ts — 116/116 pass (1 new small-buffer test).
  • bun test packages/coding-agent/test/first-run-onboarding*.test.ts (3 suites) — 33/33 pass.
  • bun run typecheck, bun run lint, bun run check:file-length — all clean.
  • Pre-commit hooks (lint, file-length, test:unit) passed on commit 95549fcb8.

Final branch state: issue-1504-impl on top of origin/main (d9b1d0ca3) with three #1504 commits: 61066df81, f0521b54b, 95549fcb8. No unstaged implementation files; only untracked research artifacts remain. No PR created.

@claude claude Bot changed the title fix: exclude workflow sessions from resume history fix(sessions): exclude workflow stage sessions from resume history Jun 25, 2026
@claude

claude Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review: exclude workflow sessions from resume history (#1510)

Solid, well-scoped change. Internal-session marking is threaded cleanly through the header type, list/continue paths, and the SDK; the opt-out semantics are correct (default-exclude, includeInternal to opt in); backward compatibility with legacy workflow sessions is documented; and docs plus both changelogs are updated. Test coverage is genuinely strong. A few observations below, mostly minor.

Code quality & correctness

  • markSessionInternal guard is nicely done. if (!header || (header.internal && header.workflow)) return; preserves a full existing marker while still backfilling workflow metadata onto a session that was marked internal but lacks linkage. Using .find(entry => entry.type === "session") rather than fileEntries[0] is appropriately defensive. 👍
  • createSessionHeader now omits parentSession when undefined (previously always set). This changes the serialized header for all sessions, not just workflow ones — headers no longer carry the parentSession key when absent. Strictly cleaner JSON and almost certainly fine, but worth confirming nothing downstream relies on the key being present (e.g. a strict equality/snapshot test elsewhere).
  • Re-marking an existing user session as internal: createAgentSession stamps internal whenever orchestrationContext.kind === "workflow-stage". If a workflow stage ever reattaches to a session that was originally a user session, this would flip it to internal and hide it from /resume. Likely not reachable today, but a one-line comment noting that stage sessions are always freshly created would future-proof the assumption.

Performance

  • Double header read in the list paths. In listSessionsFromDir / listAllSessions the prefilter calls readSessionHeader(file) (synchronous openSync/readSync/closeSync + a Buffer.allocUnsafe(64KB)), then buildSessionInfo does a full async readFile that re-parses the same header again. Because readSessionHeader is fully synchronous, the Promise.all does not parallelize it — the header reads run serially up front and block the event loop. The prefilter only saves work when internal sessions are actually present; for the common all-user-session listing it is pure overhead (one extra syscall + 64KB alloc per file). Negligible for typical counts, but you could pass the already-read header into buildSessionInfo (skipping the redundant parse) so the header is read once regardless. Not blocking — just flagging the tradeoff.
  • The readSessionHeader chunked rewrite is correct, and the decoder-flush guard (only decoder.end() on EOF-without-newline) is a real fix — the inline comment explaining why is appreciated.

Security

  • No new attack surface. JSON.parse remains wrapped in try/catch returning null, no new external/untrusted input. Buffer.allocUnsafe is used safely (only subarray(0, bytesRead) is read).

Test coverage

Excellent breadth: marking, marker preservation/backfill, findMostRecentSession/list/listAll default-exclude + includeInternal opt-in, long (>512B) headers, multi-chunk (80KB) headers, >1MiB header decoder-flush, no-trailing-newline single-line file, and the malformed-body prefilter (proving it skips before the parse that would throw). Two small gaps:

  • No test exercises SessionManager.continueRecent(..., { includeInternal }) (the wrapper), only findMostRecentSession directly.
  • markSessionInternal persistence path (_rewriteFile after flush) is only asserted via in-memory getHeader() — no round-trip re-reading the marker from disk. Worth one assertion given the if (this.flushed) this._rewriteFile() branch.

Note

The new internal-sessions.test.ts imports from vitest, consistent with the coding-agent package convention (the upstream-pi fork uses vitest across ~268 test files), as distinct from the root CLAUDE.md bun:test guidance that applies to first-party packages/* extensions. Correct for this package.

Nice work overall — the fix is targeted and the regression coverage is thorough.

Reviewed by Claude (Opus 4.8)

@mintlify

mintlify Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
bastani 🟢 Ready View Preview Jun 25, 2026, 3:51 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@flora131
flora131 merged commit 8cb16f7 into main Jun 25, 2026
11 checks passed
@flora131
flora131 deleted the issue-1504-impl branch June 25, 2026 23:39
lavaman131 pushed a commit that referenced this pull request Jun 29, 2026
…1510)

* fix: exclude workflow sessions from resume history

Mark workflow-stage sessions as internal and filter them from standard resume/list/continue paths by default, while preserving includeInternal opt-ins for workflow resume and direct session access.

Refs: #1504
Assistant-model: OpenAI ChatGPT

* fix: prevent header decoder flush corruption and prefilter internal sessions

- readSessionHeader no longer calls decoder.end() after a newline is
  found, preventing extra bytes from data after the header line from
  corrupting the parsed header (affects >1MiB headers spanning multiple
  read chunks).
- listSessionsFromDir and listAllSessions now prefilter internal sessions
  via readSessionHeader before the expensive buildSessionInfo transcript
  parse, preserving includeInternal opt-in behavior.
- Added regression tests for >1MiB header newline handling, single-line
  files, and malformed-body internal sessions.

Refs: #1504

* perf: use small dedicated header buffer for readSessionHeader

readSessionHeader now reads in 64KB chunks via a dedicated
HEADER_READ_BUFFER_SIZE instead of the 1MiB transcript buffer.
This keeps the early internal-session prefilter cheap while still
reading the full first line across chunks. Decoder.end() remains
guarded to only flush on EOF without a newline.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exclude workflow sessions from standard resume history

1 participant