feat(mt#5086): Stamp session.started with the creating actor id for the peer advisory

[minsky-ai]

Summary

warn-peer-task-activity could only say "a session.started you may not have caused", because the row carried no writer. On 2026-09-10 a conversation read exactly that as its own subagent's doing — the session had been started three minutes after it dispatched an opinion subagent — when a different, principal-launched conversation working a second succession package had created it. That attribution reached five durable records before the subagent's transcript falsified it (mt#5055, CLOSED with the PROBLEM STATEMENT FALSIFIED banner). This PR makes the inference unnecessary.

Key Changes

• session.start records who created the session. A hidden, server-injected callerActorId parameter (cliHidden + mcpHidden, same shape as tasks.claims.release's) is added to sessionStartCommandParams; session.start joins CALLER_ACTOR_ID_TOOL_NAMES in src/mcp/server.ts; emitSessionStartedEvent writes resolveCallerActorId(callerActorId) as the event's actor, and null stays null — never a fabricated id.
• The advisory says whose it is. TaskEventRow gains actor; a new pure relateSessionActor compares a conversation-scoped writer id (…:conv:<uuid>) against the hook input's session_id and decidePeerActivity renders started by <actor> (this conversation) / (ANOTHER conversation — not yours, not your subagent's) / (comparison to your own id not made — see below) / no writer id on this row. When a row is another conversation's, a paragraph states why that rules out a subagent: a subagent's MCP calls carry its parent's id (resolveLiveConversationAgentId takes only the harness pid and the spawn-time env — verified in source during planning). When the id is proc:-scoped (the shim's fallback with no pid→conversation mapping), the hook prints it, says the comparison was not made, and points at the reader's own claimedBy. Every fire now closes by naming the falsifier for "my subagent did it": <session-dir>/subagents/agent-<id>.jsonl.
• Nothing else moves. The cwd-based self-suppression runs first and is unchanged; task.status_changed rows stay unattributed; the advisory still never denies (mt#4788 owns the posture flip and is sequenced to consume this field). decidePeerActivity gains a trailing optional parameter, so turn-end-stale-state-assertion-scan's call is untouched. One static import into the hook — conversationIdFromAgentId from agent-identity/format.ts, a leaf module (format.ts → kinds.ts → nothing), outside what domain-bootstrap.ts layer 1 guards.
• Docs: docs/architecture/hooks/warn-peer-task-activity.md gains the rendering table and incident; hook-observers.mdc entry updated and recompiled (.claude/rules/hook-observers.md, .cursor/rules/hook-observers.mdc both carry it — verified by grep, not by exit code).
• Spec reconciliation: SC2's label wording ("another process" / "this process") was amended in the spec to the conversation grain the writer id actually carries; the amendment and its basis (planning audit premise (i)) are recorded on the criterion itself.

Testing

Execution evidence:

• bun test --preload ./tests/setup.ts --timeout=15000 ./.minsky/hooks/warn-peer-task-activity.test.ts → 27 pass / 0 fail (8 new cases under mt#5086 — a session.started row names its writer…).
• AT2 — relateSessionActor pins each branch; AT2 — a row started by ANOTHER conversation is labelled so, and the subagent inference is ruled out (replays the 00:14Z row with conversation e7da3c7d's id against reader dd1a36b5); AT2 — a row started by THIS conversation is labelled so; AT2 — a row with actor null renders the unattributed form; plus the proc: "not compared" case and the no-caller-id case.
• AT3 — the cwd-based self-suppression still wins over the actor label, and every pre-existing case (mt#4439 replay, PR feat(mt#4494): Give the parallel-work probe sequence a member that answers #3281 R1 window/suppression cases) unchanged and passing.
• AT4 — live tools/list on a server built from this branch: session.start params are sessionId,task,description,branch,repo,quiet,noStatusUpdate,skipInstall,packageManager,recover; callerActorId advertised: false. Contract-level: the collectMcpHiddenParamKeys machinery is mt#4579's, exercised by src/mcp/mcp-hidden-params.test.ts.
• AT1 — see ## Live verification: a session.start over MCP against this branch's server wrote a session.started row with actor populated with the id the server resolved for that client.
• SC1 — AT1's row (actor: "unknown:hash:e988ed544601b00c" — the Layer-1 id for a raw-SDK client whose name is no known harness; the resolver's OUTPUT is what is stamped, whatever scope it yields) and the CLI-path null branch in emitSessionStartedEvent (...(actor ? { actor } : {})). SC2 — the AT2 label cases. SC3 — SC3 — every fire names the subagent transcript as the falsifier, even a plain status-change fire. SC4 — SESSION_START_TOOL_NAME in CALLER_ACTOR_ID_TOOL_NAMES + AT4.
• bun scripts/run-related-tests.ts over the four changed source files → 491 pass / 0 fail across 34 files plus 46 pass / 0 fail (src/mcp/server.test.ts, basic-commands.test.ts, claims-release.test.ts, session-parameter-family-parity.test.ts, hook-module-inventory.test.ts, interceptor-coordinates.test.ts, turn-end-stale-state-assertion-scan.test.ts among them).
• R1 (registration invariant, BLOCKING) — src/mcp/server-tool-name-resolution.test.ts gains mt#5086 — session.start is REGISTERED under its dotted id, so an alias caller still hits the callerActorId injection: it builds the real createSessionStartCommand, registers it through CommandMapper.addCommand({ name: command.id }), asserts tools.get("session_start").name === "session.start", then calls the tool as session_start over an in-memory client with a spoofed callerActorId and asserts the handler received a resolved id instead. The chain it cites: registerToolsCommandsWithMcp → addCommand({ name: command.id }) (shared-command-integration.ts), normalizeMethodName strips only [^a-zA-Z0-9._-] (a dot survives), addTool maps both spellings to one object. Run: 5 pass / 0 fail. src/mcp/mcp-hidden-params.test.ts gains a case over the REAL sessionStartCommandParams (collectMcpHiddenParamKeys → ["callerActorId"]): 11 pass / 0 fail.
• validate_typecheck (session): 0 errors across 8 projects. ESLint on the changed source files: clean (a prefer-template error and two magic-string warnings were fixed before commit).

Negative control — R1's alias-injection test: with SESSION_START_TOOL_NAME commented out of CALLER_ACTOR_ID_TOOL_NAMES, the same test fails with Expected: not "spoofed-by-caller" (the caller's value reached the handler); restored, 5 pass.

Negative control — the advisory: with .minsky/hooks/warn-peer-task-activity.ts reverted to main (git stash of that one file) and the same peer-conversation row passed to decidePeerActivity with the reader's conversation id, the pre-fix message contained neither started by nor ANOTHER conversation — it rendered session.started 3m ago — session bc12bbc4-… and nothing about the writer. The new test file against the reverted hook: 0 pass / 1 fail (import of relateSessionActor missing).

Live verification

Server built from this branch (bun run <session>/src/cli.ts mcp start --http --port=39111, cwd = main workspace, throwaway static bearer token), driven with @modelcontextprotocol/sdk's StreamableHTTPClientTransport (client name mt5086-at1-smoke):

session.start {task: "mt#5055", noStatusUpdate: true, skipInstall: true}  → session 0cee7472-75ef-4605-9c79-396eb39f1a45
events.list {relatedTaskId: "mt#5055", eventType: "session.started"}:
      "actor": "unknown:hash:e988ed544601b00c",
      "relatedTaskId": "mt#5055",
      "relatedSessionId": "0cee7472-75ef-4605-9c79-396eb39f1a45",
      "createdAt": "2026-09-11T01:28:05.280Z"

The scratch session was deleted afterwards (session_delete, override reason recorded); mt#5055 stayed CLOSED (noStatusUpdate). The one session.started row on mt#5055's ledger is the residue of this check.

Deploy verification: src/mcp/server.ts, basic-commands.ts and session-parameters.ts are deploy surface (isDeploySurfaceFile → true; the seven hook/rule/doc files → false). After merge I will run deployment_wait-for-latest for minsky-mcp with notBefore = the merge timestamp and expectCommitSha = the merge sha, read buildIdentity, and assert the health body's service is minsky-mcp. Local daemon: the next session_start from any conversation after the rebuild should write an actor; I will read one back via events_list.

Sequencing

mt#4788 (flip this advisory to DENY) wants the field this PR adds; its spec now carries a sequencing note. PR #3412 (614 log-site rewrite, stale since 2026-09-04) touches basic-commands.ts and server.ts mechanically; recorded as proceed-acknowledged in mt#5086's planning audit — whoever moves second rebases, and the hunks do not intersect.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VuuiE8iQYtmaajWTbJmhzK

[minsky-reviewer]

Minsky Reviewer Status

Verdict: APPROVED — no blocking findings
Review: View review
Model: openai/gpt-5 | Tokens: 755K prompt, 7K completion | Duration: 90s
Mode: normal

Commands

• /review — request a fresh review

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Requesting one fix before merge. Overall, the PR is well-shaped: it injects a hidden caller identity for session.start, stamps actor on session.started, and renders/labels it in warn-peer-task-activity; tests and docs were updated. However, there is a blocking risk that callerActorId may not be injected when the tool is invoked under the underscore alias: the Set in src/mcp/server.ts matches only the resolved dotted tool.name. Please either prove (by citing the registration) that session.start's registered .name is always dotted or add the underscore alias to the allowlist to eliminate the silent miss. Non-blocking: consider guarding the static domain import in the hook, clarify the case-insensitivity comment in relateSessionActor, explicitly check actor !== null when emitting, and add/point to a test that tools/list omits callerActorId for session.start. Once the registration/injection invariant is settled, this is ready.

Findings

• [BLOCKING] src/mcp/server.ts:257 — Risk of missed callerActorId injection if session.start is registered under underscore alias; allowlist matches only dotted tool.name.
  The allowlist CALLER_ACTOR_ID_TOOL_NAMES (src/mcp/server.ts:257-303) includes SESSION_START_TOOL_NAME = "session.start" and matches against the resolved tool.name only. If any code path registers the tool with the underscore alias ("session_start") as the canonical name, tool.name would be "session_start" and the injection would be skipped silently, breaking SC1 for MCP calls. Fix: either (a) cite and enforce that registration for session.start always uses the dotted name so tool.name is dotted, or (b) include the underscore alias in the Set for this tool (and/or reintroduce alias mapping at the match site) to guarantee injection regardless of registration shape.

Documentation impact

• updated-in-pr — Documentation was updated in this PR alongside the code. The diff shows new/modified docs describing the new actor-labeling behavior for warn-peer-task-activity: docs/architecture/hooks/warn-peer-task-activity.md gained a section outlining the rendering cases; observer inventories were also updated in .claude/rules/hook-observers.md, .cursor/rules/hook-observers.mdc, and .minsky/rules/hook-observers.mdc. These updates cover the newly added behavior, so docs are updated-in-pr.
  Affected: docs/architecture/hooks/warn-peer-task-activity.md, .claude/rules/hook-observers.md, .cursor/rules/hook-observers.mdc, .minsky/rules/hook-observers.mdc

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Re-verified against the current spec and diff. The prior BLOCKING concern on name-resolution for callerActorId injection has been addressed: session.start is included in CALLER_ACTOR_ID_TOOL_NAMES, the CallTool path matches on the resolved tool.name, and a new test proves an alias session_start caller still receives a server-resolved callerActorId (spoofed values are overwritten). The hook now renders actor details with clear conversation-grain labeling, and the event emitter stamps actor correctly without fabricating ids. Docs were updated accordingly. I found no new critical issues introduced by the fix; one note records the positive regression test. Overall, the changes satisfy SC1–SC4 and are ready to merge.

Findings

• [NON-BLOCKING] src/mcp/server-tool-name-resolution.test.ts:146 — Good regression test proving alias callers still receive callerActorId injection
  The new test demonstrates that session.start is registered under its dotted id and that an alias session_start call is still matched against the resolved tool.name for CALLER_ACTOR_ID_TOOL_NAMES. This directly addresses the prior round's concern about name resolution. No action needed; noting as verification evidence.

Spec verification

Criterion	Status	Evidence
1. session.started rows emitted through the MCP server carry actor = the caller's resolved actor id (the same value tasks_claim returns as claimedBy for that caller). Rows emitted from the CLI carry whatever resolveCallerActorId(undefined) yields, or null — never a fabricated id.	Met	src/adapters/shared/commands/session/basic-commands.ts:214-236 — emitSessionStartedEvent now accepts actor and spreads it only when non-null; 302-315 resolves via resolveCallerActorId(params.callerActorId as string | undefined) and passes it through. src/mcp/server.ts:1193-1210, 1245-1257 — CALLER_ACTOR_ID_TOOL_NAMES includes SESSION_START_TOOL_NAME, and the CallTool handler unconditionally overwrites callerActorId with the resolved agentId so spoofing is impossible.
2. warn-peer-task-activity renders the actor beside each session.started line, and when it can resolve the reader's own actor id, labels the row another process or this process. When it cannot resolve its own id it prints the actor and says the comparison was not made — no bare "you may not have caused" for a row that names a foreign process. (Amended … labels are ANOTHER conversation / this conversation …)	Met	.minsky/hooks/warn-peer-task-activity.ts:103-156 — adds actor?: string|null to TaskEventRow, SessionActorRelation type, and pure relateSessionActor. 268-311 — per-row rendering includes — ${describeSessionActor(...)}; 301-336 — adds paragraphs for another-conversation and not-compared. 411-422 — main entry wires input.session_id to callerConversationId for comparison. Mirror changes also present in .claude/hooks/warn-peer-task-activity.ts.
3. The advisory's closing paragraph names the one cheap falsifier for the "my subagent did it" reading: the subagent transcript at <session-dir>/subagents/agent-<id>.jsonl.	Met	.minsky/hooks/warn-peer-task-activity.ts:325-336 — appended closing lines explicitly direct the reader to the subagent transcript path as the falsifier.
4. session.start is added to CALLER_ACTOR_ID_TOOL_NAMES with a mcpHidden: true callerActorId parameter, following tasks.claims.release's shape; the parameter is not advertised in tools/list.	Met	src/mcp/server.ts:1174-1193 — SESSION_START_TOOL_NAME = "session.start" and included in CALLER_ACTOR_ID_TOOL_NAMES. src/adapters/shared/commands/session/session-parameters.ts:107-137 — callerActorId param declared with mcpHidden: true and cliHidden: true. src/mcp/mcp-hidden-params.test.ts:52-65 — test asserts only callerActorId is collected as mcpHidden for the real params map. src/mcp/server-tool-name-resolution.test.ts:146-219 — proves alias session_start still receives the injected callerActorId from the server (spoofed value overwritten).

Documentation impact

• no-update-needed — Behavior change is internal/additive: stamping actor on session.started and rendering it in the advisory. The PR updates the observer doc at docs/architecture/hooks/warn-peer-task-activity.md to describe the new labels and behavior, which matches the implementation. No other user-facing CLI or API contract changed beyond hidden parameter wiring; tools/list remains silent due to mcpHidden.