Skip to content

feat(workspace): log workspace and agent state changes - #1075

Open
aqandrew wants to merge 11 commits into
mainfrom
aqandrew/devex-661-connection-state-logs
Open

feat(workspace): log workspace and agent state changes#1075
aqandrew wants to merge 11 commits into
mainfrom
aqandrew/devex-661-connection-state-logs

Conversation

@aqandrew

@aqandrew aqandrew commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

What

Add info-level logging of workspace, agent, and lifecycle status transitions so connection debugging has a record of state changes — correlated by the session ID from #1073.

This is Phase 3 of 3 for DEVEX-661 (RFC req 7). It builds on #1074.

Changes

State handling is split into three layers, one job each:

  • detectWorkspaceStateObserver / WorkspaceAgentObserver (src/instrumentation/workspace.ts) each detect a transition once and report a transition object. Stateful but effect-free (no logger/telemetry imports). The agent observer tracks every agent by ID and reports agents that disappear.
  • recordrecordWorkspaceState / recordAgentState emit the workspace.state_transitioned / workspace.agent.state_transitioned telemetry events.
  • log — inline in WorkspaceMonitor.update(), with flat scalar payloads matching the repo's other log sites. Logs state observed on first sight, state changed afterward, and agent <name> removed when an agent disappears.

WorkspaceMonitor is now the single agent-observation site, so WorkspaceStateMachine no longer tracks agent telemetry.

Important

Telemetry data change. workspace.agent.state_transitioned now covers every agent for the whole monitored session (deduped per agent), rather than only the connected agent during connection setup. In practice most workspaces have one agent, so volume is unchanged; multi-agent workspaces and flaky connections emit more. There is no sampling in src/telemetry/, so this lands as-is. EVENTS.md is updated; heads-up to whoever owns the funnel queries.

Testing

  • pnpm typecheck, targeted pnpm lint, full pnpm test (2456 passing).
  • Unit tests cover observer detection (first observation, dedup, per-agent independence, removal, reset), the record* event shapes, and the monitor's inline logging (observed vs changed, agent transitions across all agents, agent removal).
Refactor design (reviewer's three-layer shape)

Per the review, the goal was to remove the duplication between the telemetry observers and the state loggers by making transition detection happen once:

  • detect: pure-detection observers report a transition object; no logger or telemetry imports.
  • record: recordWorkspaceState / recordAgentState in src/instrumentation/, telemetry only.
  • log: inline in the monitor, same as every other logging call site.

This deletes both src/workspace/*Logger.ts files and leaves one tracker per concern.

Observer API decision: observers return a transition object (workspace → WorkspaceStateTransition | undefined; agents → { transitions, removed }) and WorkspaceMonitor.update() logs inline and calls record*. This was chosen over a constructor callback because observe() has a single synchronous caller, so a callback would only hide the side effects and complicate tests.


🤖 Generated by Coder Agents.

@linear-code

linear-code Bot commented Aug 11, 2026

Copy link
Copy Markdown

DEVEX-661

DEVEX-666

@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch from a1a27aa to 163a89b Compare August 12, 2026 22:00
@aqandrew
aqandrew requested review from EhabY and code-asher August 12, 2026 22:35
@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch from 163a89b to 082b4bb Compare August 13, 2026 00:40
Comment thread src/workspace/workspaceStateLogger.ts Outdated
Comment thread src/workspace/workspaceStateLogger.ts Outdated
@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch from 082b4bb to 163a89b Compare August 13, 2026 18:46
@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch from 163a89b to d29fcbd Compare August 13, 2026 20:26
@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch 2 times, most recently from 7f7439a to 370185b Compare August 13, 2026 23:51
@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch from 370185b to b69ba41 Compare August 13, 2026 23:54
@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch from b69ba41 to 4e8b8c8 Compare August 14, 2026 00:02
@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch 5 times, most recently from 3c2be68 to 1fc0b0b Compare August 14, 2026 00:59
@EhabY

EhabY commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Should we include an entry in the changelog for the past 2 PRs + this? We only mention user-visible changes and these are worthy of it since it means sharing the logs or browsing them would give them an easy way to correlate logs to sessions. (also more logs so they can easily debug more)

@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch from 1fc0b0b to 0991842 Compare August 17, 2026 20:21
Base automatically changed from aqandrew/devex-661-session-id-requests to main August 17, 2026 20:49
@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch from 0991842 to b1a22a8 Compare August 17, 2026 20:49
@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch 2 times, most recently from c7dce24 to 6cabf20 Compare August 18, 2026 16:26
@aqandrew
aqandrew requested review from EhabY and removed request for code-asher August 18, 2026 16:35

@EhabY EhabY left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup, the split into two loggers resolves my second comment. My first one about duplication is only half done, and fixing it properly is the one structural change I'm after, left inline on workspaceMonitor.ts:145.

Shape I have in mind, three layers with one job each:

  • detect: the observers, pure, no logger or telemetry imports, report a transition object
  • record: recordWorkspaceState / recordAgentState in src/instrumentation/, telemetry only
  • log: inline in the monitor, same as every other logging call site here

Sorry this got a bit out of hand, I saw an opportunity for some refactoring and unifying :)

Comment thread src/workspace/workspaceMonitor.ts Outdated
Comment thread src/workspace/workspaceAgentLogger.ts Outdated
Comment thread src/util/transitionTracker.ts Outdated
Comment thread src/workspace/workspaceStateLogger.ts Outdated
Comment thread src/workspace/workspaceAgentLogger.ts Outdated
Comment thread src/workspace/workspaceAgentLogger.ts Outdated
@aqandrew
aqandrew requested a review from EhabY August 19, 2026 02:50
Comment thread src/instrumentation/workspace.ts Outdated
Comment thread src/instrumentation/workspace.ts Outdated
Comment thread src/instrumentation/workspace.ts Outdated
Comment thread src/workspace/workspaceMonitor.ts Outdated
Comment thread src/instrumentation/EVENTS.md Outdated
Add WorkspaceStateLogger, which logs workspace, agent, and lifecycle
status transitions at info level with the old and new values. Wire it
into WorkspaceMonitor so state changes observed while tracking a
workspace are recorded (tagged with the session ID) for connection
debugging.
…gers

Address review feedback on the combined WorkspaceStateLogger: it logged
once per agent for a single workspace-status change and duplicated the
transition-detection already in the telemetry observers.

- Add a shared TransitionTracker and use it in WorkspaceStateTelemetry and
  WorkspaceAgentTelemetry (no behavior change).
- Split the logger into WorkspaceStateLogger (one log per workspace status
  change) and WorkspaceAgentLogger (per-agent status/lifecycle), mirroring
  the two telemetry observers. Both are wired in WorkspaceMonitor.
Mirror WorkspaceStateTelemetry: key the workspace state logger on status,
transition, and reason, and include transition and reason in the log
payload so a state-change line matches the telemetry event.
…lemetry

Reshape workspace/agent state handling into three layers, one job each:

- detect: WorkspaceStateObserver / WorkspaceAgentObserver report a
  transition object; stateful but effect-free (no logger/telemetry imports)
- record: recordWorkspaceState / recordAgentState emit telemetry
- log: inline in WorkspaceMonitor.update, matching the repo's other log sites

Deletes the parallel WorkspaceStateLogger / WorkspaceAgentLogger classes and
the WorkspaceStateTelemetry / WorkspaceAgentTelemetry classes. Agent state is
now observed once in WorkspaceMonitor for every agent over the whole session,
so WorkspaceStateMachine no longer tracks agent telemetry. Logs use flat
scalar payloads and say "state observed" on first sight, "state changed"
after, and log when an agent disappears.
The event now covers every agent for the whole monitored session (deduped),
not just the connected agent during connection setup, and transitions are
sampled from the workspace event stream so intermediate hops may coalesce.
Per CONVENTIONS.md, src/instrumentation is telemetry-only. The pure
detection code has no telemetry or logger imports, so move TransitionTracker,
WorkspaceStateObserver, WorkspaceAgentObserver, and the transition types to
src/workspace/observers.ts. The record* functions stay in instrumentation and
import the transition types from there. Observer/tracker tests move to
test/unit/workspace/observers.test.ts.
@aqandrew
aqandrew force-pushed the aqandrew/devex-661-connection-state-logs branch from 0c10123 to 8400c8b Compare August 19, 2026 23:46
Drop the generic TransitionTracker. It was only used by the two observers,
and the shared wrapper forced the awkward "nothing changed" / "first
observation" double-check. Each observer now keeps its own previous-state
memory and compares inline, shaping its result directly.
@aqandrew
aqandrew requested a review from EhabY August 20, 2026 03:23

@EhabY EhabY left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shape looks right now, the three layers each do one job and the earlier duplication is gone. Just some small clean up left 🙏

Changelog. Still missing. The Unreleased section has no entry for the session ID or the state logs.

src/instrumentation/CONVENTIONS.md:9. The checklist says one instrumentation class per domain wrapping TelemetryService, and this PR replaces two such classes with the stateless recordWorkspaceState and recordAgentState functions. Either keep the methods but update the conventions or convert them into a class if there's some shared state that is useful.

};
}

public reset(): void {

@EhabY EhabY Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both reset() methods are dead, this one and :177. Nothing in src/ calls either; the only callers are test/unit/workspace/observers.test.ts:106 and :211. Delete both methods and their two tests, since buildStartedAtMs and the maps now live and die with the monitor.

*/
export class WorkspaceAgentObserver {
/** Previous observed state per agent ID, tracked independently. */
private readonly previous = new Map<string, ObservedAgentState>();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previous and names (:132) always hold the same keys: a first sight always reports a transition, so both get set together, and the removal loop deletes from both. Fold the name into the state and drop names:

interface ObservedAgentState {
	readonly name: string;
	readonly status: WorkspaceAgentStatus;
	readonly lifecycleState: WorkspaceAgentLifecycle;
	readonly observedAtMs: number;
}

The removal loop then iterates previous. One caveat: a rename with no state change keeps the old name, which can't happen without a rebuild handing out new IDs.

readonly durationMs: number | undefined;
}

export interface RemovedAgent {

@EhabY EhabY Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RemovedAgent wraps a single name. Make it readonly removed: string[], so the monitor reads for (const name of removed) and the test asserts ["second"]. Also no need to export if this is only used internally

}

/** Reported by `WorkspaceAgentObserver`. */
export interface AgentStateTransition {

@EhabY EhabY Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AgentStateTransition is the last nested payload left. Both consumers flatten it on arrival. Put statusFrom, statusTo, lifecycleFrom, and lifecycleTo on the transition itself.

}
this.previous = { status, buildTransition, buildReason, observedAtMs: now };

const wasProvisioning =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildStartedAtMs is set only while provisioning and cleared as soon as it isn't, so wasProvisioning is exactly buildStartedAtMs !== undefined and both locals can go:

let buildDurationMs: number | undefined;
if (PROVISIONING_STATUSES.has(status)) {
	this.buildStartedAtMs ??= now;
} else if (this.buildStartedAtMs !== undefined) {
	buildDurationMs = now - this.buildStartedAtMs;
	this.buildStartedAtMs = undefined;
}

if (!transition) {
return;
}
const verb =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same ternary at :170. One module-level helper covers both and reads better at the call site:

const stateVerb = (from: string | undefined) =>
	from === undefined ? "state observed" : "state changed";

};
}

describe("telemetry", () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block now re-tests the observers. Dedup on reason (:132), build-duration resolution (:173), and the duration measurements (:96) are all observer behavior, and observers.test.ts:56, :75, and :90 cover each of them directly. Now that detection lives in one place, keep the monitor tests on wiring only: the first update logs and records once, a later change logs and records again. That drops roughly 100 lines of triple coverage.

});
});

it("dedupes an unchanged agent", () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WorkspaceStateObserver has "reports a change when only transition or reason changes" (:75), but the agent side never changes lifecycle on its own. Add the symmetric case: same status, new lifecycle_state, one transition reported.

});

describe("agent state", () => {
const agentWorkspace = (

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agentWorkspace here and workspaceWith at observers.test.ts:21 are the same builder, and this one types its agents as Array<Parameters<typeof createAgent>[0]>. Move one into the shared test mocks and have it take WorkspaceAgent[].

);

stream.pushMessage({
type: "data",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also at :330. These hand-roll { type: "data", data } instead of using the file's own workspaceEvent helper (:34). Let that helper take a Workspace as well.

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.

2 participants