feat(environment): capture multi-source environment logs with owned cleanup - #47
Closed
alvarohulse wants to merge 11 commits into
Closed
alvarohulse wants to merge 11 commits into
alvarohulse wants to merge 11 commits into
Conversation
Own tmux panes, direct processes, and file tails with exact cleanup identities so verification sessions retain bounded, recoverable environment evidence. Co-authored-by: cursoragent <cursoragent@cursor.com>
Use Cursor's personal skill directory and preserve legacy rule customizations only after the replacement skill is written successfully. Co-authored-by: cursoragent <cursoragent@cursor.com>
…nd teardown paths
…rowing type error
…onnection config keys
alvarohulse
marked this pull request as draft
August 10, 2026 21:50
…capture-health reporting
…ale artifact facts
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.
Intent
Contribute multi-source environment capture to upstream ProofShot from current upstream main. Support safely owned tmux panes, attach-only external tmux sessions, direct process stdout and stderr, file history and live tails, readiness checks, bounded canonical evidence, exact immutable process and socket cleanup, and persisted recovery state when cleanup is incomplete. Also replace Cursor's global ProofShot rule installation with a personal Cursor skill while preserving custom legacy rule content only after the new skill is written successfully. Keep viewer, verdict, publication, and global session-inventory changes out of scope. The process-ownership primitives overlap the lifecycle foundation proposed in upstream PR 45 only where required for safe environment cleanup. Revalidate the existing open PR and regenerate its body using the deployed detailed pipeline-evidence renderer.
Notes
Scope note: exact process-ownership helpers overlap the lifecycle foundation proposed in #45 because multi-process and tmux cleanup cannot safely rely on reusable PIDs, ports, or names. This PR does not include the session registry/recovery commands, viewer/verdict work, or publication changes from the larger fork.
What Changed
src/environment/module andenvironment/logsconfig keys that capture evidence from owned tmux panes, attach-only external tmux sessions, direct processes (stdout and stderr kept separate), and file history plus live tails — with readiness checks, ANSI normalization, and bounded output written to a canonicalenvironment.ndjsonplus per-sourcelogs/*.log.startlaunches and validates this environment (rejecting--runcombined withconfig.environment), andstopreports captured sources inSUMMARY.md..session.jsonand adds process-ownership helpers insrc/utils/process.ts, sostart --force,stop, andcleanrelease owned resources throughreleaseActiveSessionEnvironment, retain recovery state when cleanup is incomplete (loading config vialoadConfigForTeardownso an invalid config cannot strand resources), and makestopexit non-zero after detecting a mid-session capture gap.~/.cursor/rules/proofshot.mdc) with a personal skill at~/.cursor/skills/proofshot/SKILL.md, renaming any existing legacy rule to.migratedonly after the new skill writes successfully; adds unit and real-tmux integration tests (with a CI tmux install step) and refreshes the README, architecture, and docs pages.Risk Assessment
✅ Low: The final delta is a single correctly-placed line plus a targeted regression test and a doc update, and every finding raised across four review rounds is now resolved with source-verifiable fixes and coverage.
Testing
I re-ran the targeted suites for the changed areas and hit two failures up front: the tmux history/live assertion in
runtime.test.tsand the detached-process ownership test inprocess.test.ts. Both turned out to be test timing fragility rather than product bugs — a diagnostic run measured real capture-attach latency at ~0.4s against the test's 2s window, and the ownership test legitimately needs more than vitest's 5s default because ownership checks shell out topson every poll — so I made the live marker repeat with a live-segment-aware wait and gave the ownership tests the repo's standard 15s timeout; the suites then passed, including a re-run under deliberate CPU load. For product evidence I built the CLI and drove realproofshot start/exec/stopsessions: a tmux run capturing two owned panes plus a file tail with HTTP readiness, a direct-process run showing stdout and stderr kept apart, an attach-only run against a pre-existing user tmux session that survived stop untouched, a killed-pane run that recorded a capture gap and exited non-zero, a readiness failure that aborted start and released everything, and a bounded-evidence run that truncated at the configured byte budget. I also installed the Cursor skill into a sandboxed HOME, confirming the legacy rule is preserved only after the skill is written and stays active when that write fails. Cleanup was verified against the exact recorded socket, server, pane, and worker identities. Only the persisted-recovery-state-on-failed-cleanup path lacks end-to-end evidence; it remains unit-tested.Evidence: Evidence index for all end-to-end runs
Evidence: Multi-source capture: 3 sources, history vs live, groups, ANSI stripped
sourceId | title | group | stream | segment => events frontend-web | Web server | frontend | pty | history => 2 backend-api | API worker | backend | pty | history => 2 worker-file | worker.log tail | backend | file | history => 2 frontend-web | Web server | frontend | pty | live => 9 backend-api | API worker | backend | pty | live => 65 worker-file | worker.log tail | backend | file | live => 64 total events: 144 events still containing an ANSI escape: 0 events flagged captureGap: 0 --- frontend-web history --- [history] [frontend/frontend-web/pty] t=null [web] listening on http://127.0.0.1:4173 [history] [frontend/frontend-web/pty] t=null [tmux history/live capture boundary] --- frontend-web live --- [live] [frontend/frontend-web/pty] t=0.58s [web] GET /health [live] [frontend/frontend-web/pty] t=3.73s [web] GET / --- worker-file history --- [history] [backend/worker-file/file] t=null [worker] job -1 completed (before proofshot start) --- worker-file live --- [live] [backend/worker-file/file] t=0.98s [worker] job 1 completedEvidence: Exact cleanup after stop (recorded socket / server / pane identities)
recorded tmux socket : /tmp/proofshot-502/tmux/proofshot-2026-08-11_02-55-39.sock socket file exists after stop: false recorded tmux server pid : 33366 alive: false recorded pane frontend-web pid 33367 alive: false recorded pane backend-api pid 33438 alive: false --- leftover proofshot tmux sockets --- (empty) --- leftover capture workers --- noneEvidence: Attach-only external tmux session survives stop
--- BEFORE proofshot start --- server pid=38170 session=devbox pane=%0 pane_pid=38171 pane_pipe=0 ✓ Environment and log capture started --- ProofShot ownership claim over the external session --- ownsServer: false | ownsSession: false | sessionName: devbox recorded stopCommand: (none — attach-only) --- DURING session --- pane=%0 pane_pipe=1 --- AFTER proofshot stop: external server and pane survive, pipe detached --- server pid=38170 session=devbox pane=%0 pane_pid=38171 pane_pipe=0 socket still present: yesEvidence: Capture gap: stop reports the gap and exits non-zero
❌ ProofShot verification incomplete — environment capture stopped early 📚 Environment: 3 source(s) → .../logs (incomplete) Capture gaps: tmux pane %1 for "backend-api" exited or lost its capture pipe before "proofshot stop" — logs/backend-api.log is incomplete. Recorded with captureGap in .../environment.ndjson STOP EXIT=1 environment.ndjson row: {"version":1,"origin":"environment","group":"backend","sourceId":"backend-api","stream":"pty","segment":"live","relativeTimeSec":10.01,"text":"[tmux pane stopped feeding ProofShot before the session ended; later output was not recorded]","captureGap":true}Evidence: Direct process stdout/stderr kept separate
--- stderr-stream events (kept separate from stdout) --- [live] [backend/backend-api/stderr] t=2.28s [api] ReferenceError: demo failure while processing job 3 [live] [frontend/frontend-web/stderr] t=16.23s [process stopped by ProofShot] [live] [backend/backend-api/stderr] t=16.27s [process stopped by ProofShot]Evidence: Cursor skill install; legacy rule preserved only after success
BEFORE: ~/.cursor/rules/proofshot.mdc ✓ Cursor: Installed (Legacy rule preserved at ~/.cursor/rules/proofshot.mdc.migrated) → ~/.cursor/skills/proofshot/SKILL.md AFTER: ~/.cursor/rules/proofshot.mdc.migrated, ~/.cursor/skills/proofshot/SKILL.md (installed file identical to bundled skills/cursor/SKILL.md; custom legacy content intact) FAILURE CASE (skills path blocked): ✗ Cursor: Failed (ENOTDIR ... mkdir .../.cursor/skills/proofshot) AFTER: ~/.cursor/rules/proofshot.mdc <- legacy rule still active, not renamedEvidence: Bounded canonical evidence with logs.maxBytesPerSource = 4096
4006 bytes environment.ndjson 1260 bytes logs/backend-noisy.log 142 bytes logs/frontend-web.log truncation events: [backend-noisy/stdout] [ProofShot capture truncated at configured byte limit]Evidence: Readiness gate fails start and releases owned processes
✗ Failed to start environment capture: Environment readiness failed for http://127.0.0.1:4177/health: fetch failed start exit=1 --- leftover worker processes --- none --- session state file --- (.session.json removed)/var/folders/xh/qnn4w61965q9ml19t2yy0fk00000gp/T/no-mistakes-evidence/01KZQ7ED5Q9MYXN4D3SYDPBA0X/e2e-tmux/proofshot-artifacts/2026-08-11_02-55-39_multi-source-environment-capture/step-home.png)Evidence: Generated SUMMARY.md with Environment Logs and Capture Gaps sections
Pipeline
Updates from git push no-mistakes
✅ **Intent** - passed
✅ No issues found.
⏭️ **Rebase** - skipped
README.md- merge conflict rebasing onto origin/maincontent/docs/reference/configuration.mdx- merge conflict rebasing onto origin/mainsrc/commands/start.ts- merge conflict rebasing onto origin/mainsrc/commands/stop.test.ts- merge conflict rebasing onto origin/mainsrc/commands/stop.ts- merge conflict rebasing onto origin/mainsrc/environment/runtime.ts- merge conflict rebasing onto origin/mainsrc/environment/tmux.ts- merge conflict rebasing onto origin/mainsrc/environment/types.ts- merge conflict rebasing onto origin/mainsrc/index.ts- merge conflict rebasing onto origin/mainsrc/session/state.ts- merge conflict rebasing onto origin/mainsrc/utils/config.ts- merge conflict rebasing onto origin/main🔧 **Review** - 9 issues found → auto-fixed (3) ✅
src/utils/process.ts:138- captureBootToken() runssysctl -n kern.boottimewithout pinning TZ, and that output ends in a locale/timezone-formatted date (verified locally: the same boot printsTue Aug 4 10:41:45 2026,16:41:45under TZ=UTC, andWed Aug 5 01:41:45under TZ=Asia/Tokyo). The bootId is embedded in every recorded ProcessIdentity, and processIdentitiesMatch compares it with ===. Sibling capture code in the same file already guards against exactly this by passingenv: { ...process.env, TZ: 'UTC' }tops -o lstart=, so the omission is inconsistent within the module. Fix: pin TZ='UTC' for the sysctl call, or parse only thesec = <n>field.src/environment/evidence.ts:10- normalizeLogText strips CONTROL_PATTERN (which includes BEL, ^G, inside -�) before ANSI_PATTERN runs, but ANSI_PATTERN's OSC branch requires BEL as its terminator. Removing BEL first breaks OSC matching, and the fallback CSI branch then consumes only a short prefix, leaving residue spliced onto the next real log line. Verified against the actual patterns in this file:ESC]0;my-title BEL ready in 431 msnormalizes to "y-titleready in 431 ms" today, vs "ready in 431 ms" when ANSI is stripped first. Shells, npm scripts, and vite set terminal titles constantly inside tmux panes, so most pane evidence and logs/*.log are affected. The same bug is duplicated in the inlined worker normalize() at src/environment/workers.ts:42; both copies need the order swapped (ANSI first, then control chars).src/environment/tmux.ts:189-capture-pane -p -S -requests the pane's entire scrollback through tmuxExec (src/environment/tmux-command.ts:22), which calls execFileSync without a maxBuffer override, so Node's 1 MB default applies. The design explicitly budgets far more than that for history: logs.maxBytesPerSource defaults to 5 MB and historyBudget is half of it (2.5 MB), so the intended history size exceeds the transport limit. Attach-only external panes are the case that actually has deep scrollback, and users routinely raise tmuxhistory-limitto 10k-100k lines. Once the capture exceeds 1 MB, execFileSync throws ENOBUFS, startTmuxEnvironment's catch runs stopTmuxEnvironment, andproofshot startfails outright on an otherwise valid config. Fix: pass maxBuffer (at least the history budget) to tmuxExec, and/or bound the request with-S -<lines>.src/environment/tmux-launch.ts:158- startExternalTmux computes ownsCreatedSocket purely from "hinted socket existed before this start", never consultingattachOnly, which is only used to relax the stopCommand validation above it. With config{ launch: { kind: 'external-command', ... }, connection: { ownership: 'attach', socket: '/tmp/dev.sock' } }, if /tmp/dev.sock does not exist whenstartruns and the launcher brings it up, ownsServer/ownsSession become true and stopTmuxEnvironment (src/environment/tmux-cleanup.ts:79) runstmux kill-serveragainst the user's own dev tmux server. That contradicts the documented contract added in this change: content/docs/reference/configuration.mdx says "Attach-only launchers are never killed." The two documented rules in that paragraph are in tension (own-what-you-created vs never-kill-attach), so the author should pick which one wins rather than have me choose.src/commands/stop.ts:124-stopreleases the environment but never verifies that each capture worker was alive for the whole session, so a worker that died mid-session produces evidence that looks complete: no captureGap marker, no warning, and the summary still reports "N source(s)" with a link to a silently truncated log. The healthFailures field exists on TmuxEnvironmentState and ProcessEnvironmentState (src/environment/types.ts:127,137) but is never assigned or read anywhere, which suggests this check was intended and not wired up. The path is proven by the change's own test (src/environment/runtime.test.ts: 'records truncation and cleans descendants after a coordinator dies'), which SIGKILLs a capture coordinator and then asserts stopOwnedEnvironment resolves cleanly. This is the one gap that contradicts the invariant docs/architecture.md now states: "A silently dropped log source produces evidence that looks complete but is not ... so both are errors." Deciding whether that becomes a warning, a captureGap event, or a hard stop failure is a product call.src/environment/runtime.test.ts:39- Three of the new integration tests spawn real tmux servers and reach assertTmuxAvailable() (src/environment/tmux.ts:68), which throws 'tmux is required for environment.kind "tmux"' when the binary is absent. There is no skip guard and tmux is not declared as a dev prerequisite in package.json, README, or CLAUDE.md. .github/workflows/publish.yml gates the release job onnpm test, so any runner image or contributor machine without tmux fails the suite rather than skipping the tmux-specific coverage. Resolution is a choice between installing tmux in the workflow and guarding the tests (describe.skipIf), so it needs the author's call.src/environment/tmux-command.ts:41- runCommand spawns the external launcher detached, then throws when captureProcessIdentity returns null, without killing the child it just created. The pid is known and was spawned microseconds earlier, so there is no recycle risk in killing it; startDetachedWorker (src/environment/workers.ts:377) handles the identical case correctly byprocess.kill(-worker.pid, 'SIGKILL')before throwing. As written, a transientpsfailure leaves an unmanaged launcher plus whatever tmux resources it creates, with nothing recorded in .session.json - the exact orphan this module is built to prevent.src/environment/tmux-launch.ts:139- The stopCommand guard reads!attachOnly && ((!hintedSocket && !stopCommand) || (socketExistedBefore && !stopCommand)), but socketExistedBefore is initialized totruewhenever hintedSocket is undefined (line 136), so the first disjunct can never be the deciding branch. It collapses to!attachOnly && socketExistedBefore && !config.launch.stopCommandwith identical behavior.src/environment/workers.ts:392- createWorkerConfig is an exported identity function (return params) whose only caller (src/environment/tmux.ts:162) uses it to type an object literal. Exporting the WorkerConfig type and annotating the literal directly removes the indirection and the misleading 'create' name.🔧 Fix: Fix evidence normalization, tmux ownership, and capture-health reporting
2 issues (1 warning, 1 info) still open:
src/environment/runtime.ts:129- The new capture-health contract detects a mid-session gap as "pid file survives a dead process" (runtime.ts:127-129, documented in CLAUDE.md and docs/architecture.md), but the tmux pipe worker removes its pid file on a clean stdin EOF (src/environment/workers.ts:119-122), and a pane whose command exits mid-session produces exactly that EOF. Concrete path: environment.kind 'tmux', launch.kind 'panes', pane 'api' runningnpm run api; the command crashes at t+30s; tmux destroys the pane (default remain-on-exit off); pipe-pane's fd closes; the worker runs removePidFile() then exit(0); recordCaptureHealthFailures hitsif (!fs.existsSync(capture.pidFile)) continueand returns [];stopexits 0 and SUMMARY.md links logs/api.log as evidence with no captureGap row. The direct-process path does not have this hole - PROCESS_RUNNER writes '[process exited with code N]' into evidence before exiting - so the tmux path is the only one where a dead source is silent. This is the same failure class the round-1 fix was authorized to close ("rather than presenting incomplete evidence as successful"), still reachable. Earliest shared boundary that already holds what is needed: recordCaptureHealthFailures runs before stopTmuxEnvironment detaches the pipes, and TmuxPaneState already carries paneId and captureAttached, so verifying each recorded pane still exists with#{pane_pipe}still 1 closes it there rather than adding a second symptom patch. The worker itself cannot distinguish this EOF from the stop-time detach, so pushing the check into the worker would not work.src/environment/tmux.ts:74- releaseUnverifiedTmux still readsconfig.launch.stopCommandunconditionally (tmux.ts:72-76) and runs it on the failed-start path, without the attach guard that createTmuxState gained in this commit (config.connection?.ownership !== 'attach'). validateConfig now rejects stopCommand alongside ownership 'attach', so the CLI path is safe, but startOwnedEnvironment is exported as public API from src/index.ts:14 and accepts an EnvironmentConfig directly, bypassing validateConfig entirely. A programmatic caller passing{ ownership: 'attach', stopCommand: ... }would have its user-owned environment shut down on a start failure, contradicting the newly documented rule that attach-only environments are never terminated. Adding the same one-line ownership check here keeps the two ownership-sensitive teardown paths consistent.🔧 Fix: Detect dead tmux panes; guard attach-only unverified teardown
1 warning still open:
src/environment/tmux-cleanup.ts:51- The new pane-health signal is only valid before teardown detaches the pipes, but nothing marks panes as detached, so a secondstoprun reports every pane as a capture gap that never happened.captureAttachedis set true at attach (src/environment/tmux.ts:258,263) and never cleared anywhere; the detach loop at tmux-cleanup.ts:51-59 runs at the top of the teardown block, before every step that can push an error. Concrete path: (1)stoprun 1 - recordCaptureHealthFailures returns clean, stopOwnedEnvironment detaches all pipes, then a later step fails (e.g. 'Log helper for api did not stop.' at tmux-cleanup.ts:75, or kill-session, or the owned-server termination); the AggregateError reaches src/commands/stop.ts:141, which calls saveSession with captureAttached still true and exits 1 telling the user to runproofshot stopagain. (2) Run 2 - findUnpipedPanes (src/environment/tmux.ts:49) sees the server identity still matching (typically the same condition that made run 1 fail) and the socket matching, so it queries each pane and reads#{pane_pipe}= '0' for all of them, because run 1 detached them. Every pane is reported as 'exited or lost its capture pipe', fabricated captureGap rows are appended to environment.ndjson, healthFailures is set, and stop exits non-zero a second time. This inverts the invariant the feature exists to protect - canonical evidence now asserts a gap that did not occur - and it lands on the documented recovery flow (content/docs/reference/cli.mdx: 'Resolve the reported resource and runproofshot stopagain').start --forceandcleanreach the same state through releaseActiveSessionEnvironment. Earliest shared boundary: the detach loop already iterates exactly these panes and mutates the same state object stop.ts persists on failure, so settingpane.captureAttached = falseafter each successfulpipe-panedetach closes it there and mirrors how the flag is set on attach - no second check elsewhere is needed.🔧 Fix: Clear pane capture flag on detach to prevent false gaps
✅ Re-checked - no issues remain.
✅ **Build** - passed
✅ No issues found.
src/session/teardown.ts- The "persisted recovery state when cleanup is incomplete" branch of the intent is covered only by unit tests (start.test.tsretains recovery state / refuses forced override,stop.test.tsretains session state on cleanup failure). I could not force a real teardown failure end-to-end without an unkillable process, so there is no CLI-level artifact for that path. Everything else in the intent has end-to-end evidence.src/environment/runtime.test.ts- existing test file modified by agent: src/environment/runtime.test.tssrc/utils/process.test.ts- existing test file modified by agent: src/utils/process.test.tsnpx vitest run src/environment src/commands/start.test.ts src/commands/stop.test.ts src/commands/install.test.ts src/utils/config.test.ts src/utils/process.test.ts src/utils/errors.test.ts(52 tests, all pass after the test fixes)npx vitest run src/environment/runtime.test.ts src/utils/process.test.tsre-run under 6 concurrent CPU-load workers to confirm the de-flake holdsnode attach-timing.mjs— measured tmux pane capture-attach latency (~0.4s) to prove the 2s window in the old assertion was a test race, not a product defectnpm run buildthen end-to-endproofshot start/exec open/exec screenshot/stopine2e-tmux/with 2 owned tmux panes + file tail + HTTP readinessEnd-to-endproofshot start/stopine2e-processes/with direct processes (stdout vs stderr separation) + TCP readinessEnd-to-end attach-only run ine2e-attach/against a pre-existing user tmux session; compared server pid, pane pid,#{pane_pipe}and socket before/during/afterstoptmux kill-panemid-session thenproofshot stop— verified capture-gap event, SUMMARY.md gap section, and exit code 1proofshot startwith an unreachable readiness URL — verified exit 1, no leftover worker processes, session state clearedproofshot start/stopwithlogs.maxBytesPerSource: 4096against a noisy process — verified bounded artifact sizes and the truncation eventHOME=<sandbox> proofshot install --only cursorover a legacy~/.cursor/rules/proofshot.mdc, and again with the skills path blocked, to check legacy preservation orderingzsh check-cleanup.sh+ps/socket checks confirming recorded tmux server, pane, and capture-worker identities are all released afterstopcontent/docs/guides/install-skills.mdx:52- The agent skill install-location table is duplicated inREADME.mdandcontent/docs/guides/install-skills.mdx; this change had to synchronize the Cursor row into both copies, and the docs-site copy is already drifting — it still omits the OpenCode row that README carries. Collapsing one copy to a pointer at the other (README owning the user-facing summary, the guide owning the deep detail) would stop the next agent-support change from having to edit two places. Left as-is: deduplicating cross-surface tables is a documentation-architecture change beyond this pass.issues/02-interactive-timeline-viewer.md:231-issues/02-interactive-timeline-viewer.mdlistsskills/cursor/proofshot.mdcin its files-to-change table; this change deleted that file. Theissues/directory is a historical record of completed planning work — likeproofshot-spec.md, which this change explicitly banner-marked as historical. Adding the same marker toissues/(or removing them) is a judgment call about whether that directory is durable documentation, so it was left untouched rather than expanded into a broader historical-doc sweep.src/commands/pr.ts:276-npx tsc --noEmitfails with TS2322 atsrc/commands/pr.ts:276—normalizeUploadProviderreturnsprovider || 'repo-contents'typed asstringwhereGitHubUploadProvideris required. Pre-existing:src/commands/pr.tsis byte-identical between the base and target commits, so this is not caused by this change. CI does not gate ontsc --noEmit(it runstsup+vitest), so this is latent rather than build-breaking. Not fixed here because the fix is a functional narrowing in untouched code and lies outside this change's scope.✅ **Push** - passed
✅ No issues found.