Skip to content

feat(harness): render live Agent Map proposals - #777

Merged
ynadge merged 47 commits into
mainfrom
yashnadge/sap-3064-ui-render-the-golden-path-proposal-live
Sep 3, 2026
Merged

feat(harness): render live Agent Map proposals#777
ynadge merged 47 commits into
mainfrom
yashnadge/sap-3064-ui-render-the-golden-path-proposal-live

Conversation

@ynadge

@ynadge ynadge commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Parse strict accepted-proposal deltas and project them atomically through one project-keyed loader, with durable refetches for load races, proposal or version gaps, and reconnects on the existing WebSocket.
  • Render the five-kind proposed Agent Map as a read-only, accessible canvas with semantic edges, deterministic layout, fit/pan/zoom/keyboard controls, and a structured provenance inspector.
  • Extract graph-neutral viewport and directed-layout utilities while retaining the existing SystemGraph compatibility surface.
  • Launch Agent Map planners with a dedicated architecture-planning profile so they never inherit Studio's scaffold/run/deploy coding instructions, while the raw CLI transcript still begins with the developer's real input.
  • Issue session-scoped Agent Map MCP authority to signed-out local principals as well as authenticated users.
  • Add content-free proposal telemetry, redact user-authored Agent Map labels from autocapture, a deterministic stock-research fixture, golden browser coverage, and a harness changeset.

Stack

This is the fourth E2 PR and is based on #776.

Verification

  • Full harness Vitest suite: 200 files, 3,246 tests passed
  • Focused planner, MCP, API, and privacy matrix: 6 files, 116 tests passed
  • Full Agent Map Playwright spec: 11 tests passed
  • Harness server and web typecheck
  • Harness lint
  • Harness production build, with only the existing Vite import and chunk-size warnings
  • git diff --check
  • Manual review approved the behavioral/privacy head; the final commit only relocates its requested realistic redaction test and adds compatibility deprecation docs
  • Current-head GitHub review check completed successfully (automatic substantive review skipped at the configured round cap)

The changed implementation and regression hunks match project formatting. Whole-file Prettier still reports unrelated pre-existing formatting in the four touched legacy TypeScript files; broad formatter churn was deliberately excluded.

Design and fixture evidence

Implementation follows the E2.4 journey/interface contract, existing AgentMapPane and SystemGraph anatomy, and the available neutral token contract. No new color, tone, type role, size, or permission vocabulary was added. The vendored design-system widget guide and design-source checkout were not present in this repository.

The golden browser case renders the deterministic stock-research proposal within one second, covers all five node kinds and six relationship kinds, verifies planner provenance, applies a contiguous coding-agent rename live without resetting the viewport, and preserves attribution across a second delta. No screenshot artifact is uploaded.

Planner bootstrap follow-up

Claude Code's interactive CLI has no supported hidden assistant-first turn. Studio now keeps trusted planning identity, onboarding behavior, and map-tool instructions in the existing system-prompt appendix and does not submit a synthetic control prompt through the PTY. The first visible conversation turn is therefore the developer's actual input.

Signed-out Studio uses the same stable local:<machineId> principal for planner ownership and private MCP capability issuance. A full-server regression opens a real no-auth planner, inspects its private generated config, connects through the bearer capability, and lists agent_map_read, agent_map_validate, and agent_map_propose.

Known limitations

  • The Agent Map uses the new graph-neutral directed layout, while legacy SystemGraph retains its existing SCC layout engine behind the compatibility surface rather than sharing that new core.
  • The bounded validation-failed event name is registered, but the browser has no client validation-failure source to emit it; backend validation metrics remain authoritative.
  • Canvas behavior is covered by the full golden Playwright spec rather than a dedicated component-unit suite.
  • If a transient catalog read returns an empty Studio project list, the open map recovers correctly but accepted deltas may each incur a workspace GET until the catalog recovers.
  • Loader generations intentionally remain monotonic for stale-read safety, retaining one small integer per project opened during the tab lifetime.
  • Under --no-auth, the remote sapiom MCP remains intentionally unauthenticated, and Claude may also report inherited user-level MCPs that need authentication. That banner is independent of the private agent-map server, which is now present and capability-authenticated.

Closes: SAP-3064

Linear: https://linear.app/sapiom/issue/SAP-3064

Immediate planner contract follow-up

  • The trusted map-planner role now selects a standalone planning system prompt; ordinary Claude Code and Codex sessions continue loading the served Studio coding profile.
  • agent_map_validate and agent_map_propose now advertise all six strict operation variants through MCP discovery while malformed calls still receive bounded service-authored validation and recovery details.
  • Post-follow-up verification: 5 focused files / 44 tests, harness typecheck, lint, server build, and diff checks passed.

Live proposal delivery follow-up

  • Production now publishes each durably accepted Agent Map proposal to the existing Studio event WebSocket, so an already-open map projects the change without a later navigation or planner read.
  • A full-server regression opens the real event socket, proposes through the private MCP endpoint, and requires the matching versioned delta to arrive.
  • Claude Artifact support remains unchanged; it was a useful fallback after the missing Studio event, not the cause of the rendering failure.
  • Follow-up verification: full harness Vitest suite (200 files / 3,246 tests), focused live-map matrix (5 files / 18 tests), typecheck, lint, production build, focused Prettier, and diff checks passed.

Inspector dismissal follow-up

  • The Agent Map node inspector now has the existing Studio X-style close control and supports Escape while focus is within the map.
  • Closing restores focus to the selected node, preserving keyboard navigation and avoiding a dead focus target when the sidebar unmounts.
  • The implementation reuses the existing ghost-button, icon, spacing, and token vocabulary; no new design-system values were introduced.
  • Verification: focused browser regression passed, full Agent Map Playwright file passed 11/11 with one bounded worker, harness typecheck/lint/build passed, changed TS/TSX formatting and scoped CSS formatting passed, and diff checks passed.

Planner SessionStart orientation follow-up

  • A fresh Claude Agent Map planner now shows a brief native SessionStart system notice after folder trust, explaining that the session scopes the desired build into a reviewable Agent Map and that Studio will create focused execution sessions after approval.
  • The notice is display-only: it does not create a model/user turn, expose hidden instructions, or affect ordinary and resumed sessions.
  • Follow-up verification: 5 focused files / 190 tests, harness typecheck, lint, server build, and diff checks passed.

Post-#774 stack sync

  • Merged the verified feat(harness): expose scoped Agent Map planning tools #776 parent normally and resolved the sole overlap in server launch setup by retaining both fix(harness): refresh MCP credentials at launch #778 live credential refresh and the dedicated Agent Map planner prompt.
  • The SAP-3064 diff remains the same 42-file slice, and its web tree is unchanged from the previously approved head.
  • Fresh verification: full harness Vitest (201 files / 3,263 tests), focused launch-conflict matrix (7 files / 209 tests), harness typecheck, lint, production build, and diff check.
  • Browser reruns completed 10/11 twice because Chromium returned net::ERR_INSUFFICIENT_RESOURCES while loading unchanged Vite modules in the failed-greeting case; that exact case passed 1/1 in isolation. The trace showed an empty root caused by resource fetch failures before planner behavior, not an application assertion or source regression.

Agent Map full-view follow-up

  • The shared expand control now appears beside the Agent Map panel toggle with an explicit Expand Agent Map label.
  • Expanding lifts the existing live map into the same fixed full-view frame used by per-agent graphs, preserving its pan/zoom transform and inspector state rather than remounting it.
  • The in-frame minimize control and Escape both exit full view; Escape closes an open node inspector first.
  • Follow-up verification: harness typecheck, production web build, the new Agent Map full-view browser regression, and the existing canvas expansion regression passed. The surrounding Agent Map file passed 11/12 on its bounded run; its unrelated failed-greeting startup case passed immediately in isolation.

Full-view resize isolation follow-up

  • Both workspace and canvas pane resize separators are now removed while either Agent Map or per-agent canvas is in full view, preventing hidden shell width mutations and eliminating the visible drag outlines.
  • The separators return unchanged on exit.
  • Follow-up verification: red/green Agent Map regression; 19/19 surrounding Agent Map, mobile, resize, and canvas-expand browser cases passed; harness typecheck and production web build passed; diff checks passed.

Terminology-guard follow-up

  • Rephrased the planner's implementation prohibition using Agent Studio product terminology.
  • Narrowly allowlisted the private design-system Workflow icon identifier used for Agent Map subagent nodes, consistent with existing icon exceptions.
  • Verification: the exact terminology check passed across 496 files, all 10 terminology-guard tests passed, 4 focused planner/launch files passed 70/70 tests, harness typecheck passed, and diff checks passed.

Add strict caller schemas and a pure prospective-graph validator with batch-local reference resolution, deterministic touch sets, and post-validation ID materialization.

Closes: SAP-3061
Renew live-session capabilities on authenticated use, return bounded project recovery, close failed MCP initializations, and preserve original session persistence failures. Extend full-server SDK coverage and document the embedded endpoint.

Refs SAP-3060
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review — PR #777 feat(harness): render live Agent Map proposals

Public-copy hygiene: clean. The changeset names no company, no arrangement, no internal
host or ticket, and correctly warns consumers about the widened UiEventName /
AnalyticsEventType unions. TikTok in the golden fixture is a public platform already
used as fixture vocabulary elsewhere in this repo (web/src/lib/agent-groups.test.ts), not
a leak. minor is the right level.

Findings

1. Deltas are never routed by project — a delta for project B corrupts project A's load and leaves B stale

packages/harness/web/src/lib/use-agent-map-entry.ts

useHarnessState fans agent-map.proposal.changed out to every listener with no project
filter, and the hook's listener calls
parseAcceptedProposalDelta(rawDelta, projectId) pinned to the active project. A
well-formed delta for a different project therefore throws, and the catch arm treats it
as corruption: agentMapLoader.invalidate(projectId); loadWorkspace(projectId).

Failure: a user with two Studio projects open in one session, with an agent editing
project B. Every accepted B delta forces a full GET refetch of A's workspace — a refetch
storm proportional to B's edit rate. Worse, that delta is dropped rather than applied to
B's cached snapshot, and createAgentMapLoader.load returns snapshots.get(projectId)
unconditionally when present, with no TTL. Switching back to B renders the pre-delta map
for the rest of the session — no reconnect, no gap, so nothing ever invalidates it.

Fix: read rawDelta.projectId first and dispatch to the loader for that project
(accept is already keyed by delta.projectId); reserve the parse-failure path for a
delta whose project actually matches.

2. The projector never appends applied operations to proposal.history, so the provenance inspector goes stale after the second delta

packages/harness/web/src/lib/agent-map-projector.ts:1257 (applyAcceptedProposalDelta result)

The projected snapshot bumps version, recordVersion and updatedAt but carries
proposal.history through untouched. latestNodeAttribution checks latestDelta (only
ever the single newest one) and otherwise falls back to that unchanged history.

Failure: delta 1 renames node X, delta 2 renames node Y. Select X — latestDelta is
delta 2, which doesn't touch X, so the inspector falls back to history and reports the
original planner as the last actor (or renders no "Latest change" section at all),
attributing someone else's edit to the wrong role. Structured provenance is the headline
feature of this PR; this makes it wrong in the ordinary two-edit case. The projection is
also no longer equal to what a GET returns for the same version, which undercuts the
"durable refetch" invariant the PR is built on.

Fix: append the delta's operations as history entries (ids from operationIds,
acceptedVersion: delta.version, actor, acceptedAt) inside the applied branch, and
drop the latestDelta prop threading through AgentMapPaneAgentMapInspector.

3. Every live delta refits the canvas, throwing away the user's pan and zoom

packages/harness/web/src/components/AgentMapCanvas.tsx:386

computed memoizes on [proposal.nodes, proposal.relationships]; the projector returns
fresh arrays for every applied delta, so layout is a new object, fit (dep [layout])
is a new callback, and the useLayoutEffect re-runs with a fresh initialized = false
and calls fit().

Failure: a user zooms into one corner of a large proposal; a collaborator's rename lands;
the viewport snaps back to fit-and-center mid-read. The effect is clearly written to fit
once per proposal (the initialized latch, proposal.id in the deps), so this is the
latch being defeated rather than intent. Key the one-shot fit on proposal.id in a ref
instead of on effect identity.

4. agentMapLoader is a module singleton whose eviction path is never called

packages/harness/web/src/lib/agent-map-loader.ts:1026

retain(projectIds) is written, tested only indirectly, and invoked from nowhere in the
diff. Combined with the no-TTL cache in finding 1, a long-lived Studio session retains a
full AgentMapWorkspaceResponse — nodes, relationships and complete operation history —
for every project ever opened, for the lifetime of the tab. Either call retain from the
project-list effect that already knows the live set, or delete it rather than shipping an
uncalled cache-management API.

Verdict

Request changes — findings 1 and 2 are user-visible correctness bugs in the two behaviors
the PR exists to deliver (live projection and provenance), and neither is covered by the
current tests: agent-map-loader.test.ts and agent-map-projector.test.ts both exercise
single-project, single-delta paths only.

ynadge commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all four findings in bce1a41:

  1. Valid deltas are parsed before active-project comparison and routed to the project-keyed loader; malformed foreign frames no longer invalidate the active map.
  2. Applied operations now extend projected history, and the single latestDelta prop path is removed. A two-delta regression verifies attribution for the earlier node remains correct.
  3. Initial fit is latched by proposal id, with golden browser coverage proving a live rename preserves the user viewport.
  4. The live Studio project catalog now drives loader retention. Event-only projects are not cached, and an in-flight read cannot resurrect an evicted project.

Fresh gates: focused Vitest 70/70, full Agent Map Playwright 11/11, typecheck, lint, build, and diff check all pass.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #777 (delta since 16cb6a6f)

All four earlier findings are fixed: deltas now route on delta.projectId and a foreign
delta no longer invalidates the active project; the projector appends appendedHistory and
latestDelta threading is gone (covered by a two-delta unit test and the new e2e node-102/101
case); the fit latch is keyed on proposal.id via fittedProposalRef; retain is wired to
state.studioProjects. No public-copy issues in the delta — new comments and fixture strings
are generic.

Two new findings, both introduced by those fixes.

1. retain() can evict the active project, and the new accept early-return then drops its deltas silently

packages/harness/web/src/lib/agent-map-loader.ts:106

if (!snapshot && !requests.has(projectId)) return { status: "ignored" } — the hook only
refetches on needs-refetch, so an evicted-but-open project applies nothing and shows no error.
retain is fed state.studioProjects ?? [], and listStudioProjects
(packages/harness/src/server/index.ts:2896) catches a catalog read failure and returns [].
So one transient studio-projects.json read error during any refreshWorkspaceScopes (session
create, project open) evicts the open map's cache; every subsequent accepted proposal is
silently ignored and the canvas is frozen until a WS reconnect or a project re-select.
Fix: return needs-refetch for an uncached project with no in-flight read — the hook already
discards outcomes for foreign deltas, so only the active project refetches.

2. retain deletes generations, bypassing the new in-flight guard

packages/harness/web/src/lib/agent-map-loader.ts:149

After invalidate (generation 1) → loadretain(∅), the resolving read sees
generationFor === 0 !== 1 and takes the stale-generation branch, which recursively re-GETs and
re-caches the evicted project — the requests.get(...)?.promise !== promise guard at line 66 is
never reached. Drop generations.delete(projectId) from retain to keep the counter monotonic.

Nit: the project-routing fix in use-agent-map-entry.ts still has no unit test (no
use-agent-map-entry.test.ts); it is covered only indirectly by the loader test.

Verdict: finding 1 is a silent-staleness regression in the PR's headline behavior; fix
both, then this is ready.

ynadge commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #777 (delta since bce1a41e)

One commit moved (ef08e13a, 5 files). Both round-2 findings are fixed:

  • Eviction no longer silences the active project. accept now returns needs-refetch
    instead of ignored for an uncached project with no in-flight read
    (agent-map-loader.ts:106), and the hook's delta.projectId !== projectId early-return
    still keeps foreign deltas from refetching the active map. A retain(∅) blip is now
    recoverable on the next accepted delta.
  • retain keeps the generation counter monotonic (generations.delete dropped,
    agent-map-loader.ts:146), so the in-flight guard at line 66 is reached and an evicted
    read no longer resurrects its cache. The new assertion
    (getAgentMapWorkspace called once, peek null) pins it.

The round-2 nit is also closed: the routing branch is extracted to a pure
routeAcceptedProposalDelta (agent-map.ts:231) with direct coverage for the
foreign-valid / foreign-malformed / active-malformed cases. Extraction is behaviour-
identical to the inline version it replaces.

No new findings. Public-copy hygiene in the delta is clean — code and fixtures only, no
changeset edit, and routeAcceptedProposalDelta lives in web/src, which the package's
files/exports do not ship, so the public API surface is unchanged.

Two low-severity residuals, neither blocking:

  • retain still evicts the open project when listStudioProjects returns [] on a
    transient catalog read failure; recovery is now correct, but while that failure persists
    every accepted proposal costs a full workspace GET rather than an in-memory projection.
    Skipping eviction for the active projectId would remove the whole class.
  • generations is now pruned by nothing, so it grows by one small int per project ever
    opened, for the tab's lifetime — the intended trade for correctness, worth a comment
    saying so.

Verdict: approve — the push resolved everything raised in round 2.

ynadge commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Implemented the user-confirmed Studio UI contract at 9023d9d6:

  • Agent Map planner sessions now render the existing raw Terminal/Claude Code CLI in the center; the planner transcript/composer facsimile is no longer mounted.
  • Selecting an individual agent preserves that same CLI while the right pane switches to the agent Canvas and enables Steps; returning to the project preserves the CLI while restoring Agent Map.
  • No CSS, tokens, or terminal styling changed.

Fresh verification: Agent Map Playwright 11/11, focused Vitest 70/70, harness typecheck, lint, production build, scoped Prettier, and diff check all pass.

ynadge commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #777 (delta since ef08e13a)

One commit (9023d9d6, 3 files) swaps the planner conversation pane for the raw
Terminal. The swap itself is sound: Terminal's effect keys on [sessionId, token],
so branch reuse reconnects instead of stranding the old session, and the two
agent-panel-terminal ids are mutually exclusive.

1. The changeset now describes a UI this commit deleted

.changeset/live-agent-maps-render.md — "Render the shared Agent Map proposal live
beside its planning conversation." There is no planning conversation as of
9023d9d6; the centre pane is the raw coding-agent CLI. This string compiles verbatim
into packages/harness/CHANGELOG.md and ships in the npm tarball, where it cannot be
corrected. Reword to the planning session / agent CLI.

2. ~500 lines and 50 CSS selectors orphaned; greeting failure is now silent

PlanningConversationPane.tsx (383 lines), use-planner-transcript.ts (110), 50
.planning-conversation/.planner-*/.transcript-* selectors in styles.css, and the
sendPlannerMessage/retryPlannerGreeting path through api.tsuse-harness-state.ts
have no callers left. The TS tree-shakes; the CSS does not, so it ships in every harness
bundle, and use-planner-transcript.test.ts keeps a green suite over unreachable code.
User-visible consequence: retryPlannerGreeting was the only recovery for
greeting: "failed" and App.tsx no longer reads greeting state at all — the rewritten
e2e asserts precisely that (planner-greeting-retry count 0). Delete them, or state why
they stay.

3. The golden e2e no longer proves a proposal rendering live

mockAgentMapGolden=1 seeds the fixture straight into MockApi.getAgentMapWorkspace
(api.ts:2313), so all six nodes now arrive by GET; only two update-node deltas take
the WS path that previously carried the whole proposal. add-node is unit-covered, so
this is narrowed coverage rather than a bug — but live rendering is the PR's title.
Relatedly, "greeting lifecycle state never replaces the raw planner CLI"
(agent-map-planning.spec.ts:218) only asserts three deleted testids have count 0; it
can no longer fail for the reason it names.

Unfixed from round 3, both previously logged non-blocking: retain still evicts the
open project when listStudioProjects returns []; generations is still unbounded and
uncommented.

Verdict: fix the changeset wording before merge — it is the one unretractable
surface. 2 and 3 are cleanup.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #777 (delta since 00156d53)

One commit (85271cda), one file, one line: renameDelta()renameDelta(0) in the
superseded-request case of use-agent-map-entry.test.ts.

Round-11 finding 1 is fixed and the fix is real. With fromVersion = 0 the delta's
version is 1 and proposalSnapshot().proposal.version is 1, so snapshot.proposal.id === delta.proposalId and version >= delta.version both hold; the generation mismatch
(2 !== 1) is now the only clause returning false. Deleting
currentWorkspaceRequest === workspaceRequest from shouldCommitAcceptedDelta
(use-agent-map-entry.ts:119) makes the negative test fail — the gate that churned across
5d78a65d, c10137a2, 8cedea80, 00156d53 is finally pinned. No prose surface, no
production code, no package.json touched; changeset unchanged.

No new findings. Round-11's nit (adjacent same-typed number, number params on the
predicate, so a swapped call site typechecks silently) is unaddressed and remains a nit.

Still open, all previously logged non-blocking: retain evicts the open project when
listStudioProjects returns []; generations grows unbounded with no comment naming the
stale-read trade; visibleDeltaRef / visibleProposalRef are never pruned.

Verdict: approve — the delta closes the last open finding; nothing further needed.

Keep planner onboarding in hidden launch context so the raw Claude Code transcript begins with the user's real input. Use the stable local principal for signed-out capability issuance so scoped Agent Map tools are present under --no-auth.\n\nRefs: SAP-3064

ynadge commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #777 (delta since 85271cda)

One commit (a94b284a, 5 files): greeting is skipped at creation, onboarding moves into the
planner prompt appendix, and signed-out sessions now get an Agent Map identity via
localPlanningPrincipal. No web/loader code touched, so nothing from rounds 7–12 regressed.
Changeset gains one line; it is provider-neutral and names no company — prose is clean.

1. The onboarding directive re-fires on every resume and rehydrate

planning-session.ts:204, callers at :341 and :504

"In your first response, briefly explain that you and the user can plan agents,
responsibilities, data flow, resources, and connectors together" is unconditional in
buildFocusedPlannerContext, but that appendix is rebuilt for the resume and rehydrate paths,
both of which spawn a fresh CLI. A developer reopening a project that already has a confirmed
revision gets the introductory pitch again as the planner's next turn. emptyProject is
computed two lines above and never consulted — gate the sentence on it, or on
resolution === "created".

2. The greeting delivery pipeline is now unreachable, and the changeset doesn't say so

planning-session.ts:448,497,555,574

After this commit no production path produces { status: "pending" } — only test fixtures do.
So the generate/deliver/retry half of PlannerGreetingCoordinator, POST .../planner-sessions/:id/greeting/retry, the shipped retryPlannerGreeting() client method,
and all five planner_greeting.* AnalyticsEventType members can never fire again; the
coordinator survives only as the queued-input drain. Dashboards on planner_greeting.delivered
silently go to zero on a minor. Either @deprecated those states/route or state it in the
changeset body.

Nit: the identity fallback is session-level, so under --no-auth every session in a catalog
project — not only planners, as the changeset says — now gets propose-capable Agent Map tools.
The new test covers the planner path only.

Still open, all previously logged non-blocking: retain evicts the open project on an empty
listStudioProjects; generations grows unbounded; visibleDeltaRef / visibleProposalRef
are never pruned; predicate still takes adjacent same-typed number, number.

Verdict: approve — finding 1 is a one-condition fix, finding 2 a changeset or @deprecated
line. Nothing earlier rounds reported was wrong.

Avoid replaying introductory guidance after resume or rehydration, document the compatibility-only greeting lifecycle, and cover signed-out builder capabilities.\n\nRefs: SAP-3064

ynadge commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #777 (delta since a94b284a)

One commit (e1e0053c, 4 files): onboarding sentence is gated behind a new
onboardOnFirstResponse input, and the changeset gains the greeting-lifecycle and
signed-out-scope corrections. buildFocusedPlannerContext is not re-exported from
src/index.ts, so the new required field is not a public-surface break. Changeset prose is
provider-neutral and names no company.

1. New — the onboarding gate has only negative coverage

planning-session.test.ts:172, planning-session.ts:205

The two new assertions are both not.toContain, and the direct builder case that passes
onboardOnFirstResponse: true never asserts the sentence is present (grep: the string
appears only at :468 and :492, both negated). Collapsing the ternary to "" — i.e.
dropping the onboarding directive the PR summary says it preserves — passes the whole suite.
Add a toContain on the true builder case. Same shape as the vacuous-gate finding from
rounds 10–12.

2. Round-13 finding 1 — narrowed, not eliminated

planning-session.ts:346, :574

mode === "created" excludes resume and rehydrate, which was one of the two fixes offered.
But the no-candidate fall-through at :574 is also "created", so reopening a project with
a confirmed revision after a Studio restart still gets the introductory pitch; emptyProject
is still computed and never consulted.

3. Round-13 finding 2 — changeset only, types still look live

shared/types.ts:829-833

The changeset now states the lifecycle is compatibility-only, which satisfies the ask. The
five planner_greeting.* AnalyticsEventType members and the .../greeting/retry route
carry no @deprecated, so a consumer reading types alone still treats them as active.

Correction to an earlier round: round 13 called retryPlannerGreeting() a "shipped
client method"; it lives in web/src/lib/api.ts, SPA-internal and not exported from the
package.

Still open, all previously logged non-blocking: retain evicts the open project on an
empty listStudioProjects; generations grows unbounded; visibleDeltaRef /
visibleProposalRef are never pruned; predicate takes adjacent same-typed number, number.

Verdict: approve — finding 1 is a one-line test assertion; 2 and 3 are optional polish.

ynadge commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #777 (delta since e1e0053c)

One commit (5b4e9737, 4 files). No prose surface, no package.json, no changeset change —
public-copy hygiene unaffected.

Round-14 findings 1 and 2 are both fixed. planning-session.test.ts:184 adds the positive
toContain("In your first response, briefly explain") on the onboardOnFirstResponse: true
builder case, so collapsing the ternary now fails. emptyProject is hoisted to
planning-session.ts:329 and gates the flag (mode === "created" && emptyProject), which also
covers the no-candidate "created" fall-through; the new fixture-injected
confirmedRevisionId case pins it.

1. New — the node object: "agent" is a redaction trigger, unmarked and untested

AgentMapCanvas.tsx:279, AgentMapInspector.tsx:33

"agent" is in USER_NAMED_OBJECTS (before-send.ts:103), so it selects drop_name: $el_text
deleted, attr__aria-label stripped, text="". That is the whole reason node names and the
inspector's aria-label={${node.name} details} don't reach PostHog. But the constant reads as a
description of the node, and data-node-kind={node.kind} sits on the adjacent line — the obvious
"the kinds are wrong, use node.kind" edit silently ships user-authored resource, connector,
artifact and subagent names into $el_text, since none of those four are in the set. Nothing
fails if the spread is deleted outright either: the golden Playwright spec asserts no analytics,
and before-send.test.ts has no agent-map case. A comment naming the redaction contract plus one
beforeSend test over an agent-map click would hold it. (Corollary of the same hardcode: all five
kinds report object=agent, so kind-level click breakdown is unavailable — the fix is adding the
four kinds to USER_NAMED_OBJECTS first, never passing node.kind before that.)

2. Not fixed — round-14 finding 3

shared/types.ts:829-833. The five planner_greeting.* members and .../greeting/retry still
carry no @deprecated; a consumer reading types alone still treats them as live.

Neither AgentMapPane nor any App ancestor sets surface, so these clicks are indistinguishable
from WorkflowActionsHeader.tsx:174's object: "agent" — nit, one attribute on
.agent-map-live.

Still open, all previously logged non-blocking: retain evicts the open project on an empty
listStudioProjects; generations grows unbounded; visibleDeltaRef / visibleProposalRef are
never pruned; the commit predicate takes adjacent same-typed number, number.

Nothing earlier rounds reported was wrong.

Verdict: approve — finding 1 is a comment plus one test; finding 2 a JSDoc line.

ynadge commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #777 (delta since 5b4e9737)

One commit (40914601, 5 files): privacy comments on the two agent-map components, a
surface: "agent_map" tag on .agent-map-live, an agent-map case in before-send.test.ts,
and @deprecated on the five planner_greeting.* members. No changeset, README, or
package.json change — public-copy hygiene unaffected. The @deprecated text is
provider-neutral and names no company. The surface tag works: nothing in App.tsx or any
ancestor of AgentMapPane calls trackingAttrs, so there is no outermost-wins conflict.

Correction to round 15

Round 15 claimed "nothing fails if the spread is deleted outright." That is wrong. Both
AgentMapCanvas.tsx and AgentMapInspector.tsx build an aria-label by template-literal
interpolation, so both are offenders in the static tripwire at
redaction-gate.test.ts:262-306, which requires /object:\s*"/ in the file. Deleting the
spread fails that test today — and so does the object={node.kind} edit round 15 named,
since it carries no quote. The pin round 15 asked for largely already existed.

1. The new test cannot fail for anything the component does

before-send.test.ts:112-125

It hardcodes object: "agent" in the payload, so no edit to AgentMapCanvas.tsx or
AgentMapInspector.tsx can break it, and its assertions are a strict subset of the very next
case (:127, same object, same two assertions plus more) — surface is never read or
mutated by beforeSend. It also omits $elements_chain, the carrier a remote-config flip
selects, so the text="" rewrite on an agent-map click stays untested. The case belongs in
redaction-gate.test.ts alongside the other seven surfaces, with a realistic node-button
chain and the node name as a needle.

2. Round-15 finding 2 — types fixed, route not

AnalyticsEventType now carries @deprecated. The POST .../planner-sessions/:id/greeting/retry
route and retryPlannerGreeting() still do not, so an HTTP consumer still reads it as live.

Still open, all previously logged non-blocking: retain evicts the open project on an
empty listStudioProjects; generations grows unbounded; visibleDeltaRef /
visibleProposalRef are never pruned; the commit predicate takes adjacent same-typed
number, number; all five node kinds still report object=agent, so kind-level click
breakdown is unavailable.

Verdict: approve — finding 1 is a test relocation, finding 2 a JSDoc line; neither blocks.

ynadge commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Final planner-bootstrap follow-up handoff at d06db46cf4c5f0fcd2e8b6c8702f9502f56b2b14:

  • Removed the synthetic planner control turn from the PTY path; trusted planner/onboarding instructions remain in the hidden launch appendix, and first-time onboarding is limited to a newly created empty project.
  • Made signed-out Studio sessions use the stable local:<machineId> principal when issuing private Agent Map MCP authority. The full-server regression opens the real planner route and lists agent_map_read, agent_map_validate, and agent_map_propose through the authenticated ephemeral endpoint.
  • Preserved the raw Claude Code terminal UI. There are no layout, styling, terminal-rendering, or facsimile changes.
  • Marked the retired greeting telemetry/API/retry route as compatibility-only and added realistic Agent Map autocapture redaction coverage.

Fresh final evidence:

  • focused planner/MCP/API/privacy: 6 files, 116 tests
  • full harness Vitest: 200 files, 3,246 tests
  • typecheck and lint: pass
  • production build: pass (existing Vite warnings only)
  • full Agent Map Playwright: 11/11
  • diff check: pass

The substantive follow-up review approved the behavior/privacy head. This final commit implements its last two nonblocking requests; the current automatic check completed successfully with the expected configured round-cap skip. The dedicated Mac checkout is synced and built at this exact SHA; its existing Studio process was intentionally not restarted.

…ckend-persist-one-shared-versioned-map-proposal

# Conflicts:
#	packages/harness/src/core/agent-map-proposal-schema.ts
#	packages/harness/src/core/agent-map-proposal-validator.ts
#	packages/harness/src/shared/agent-map.ts
…st-one-shared-versioned-map-proposal' into yashnadge/sap-3060-infrastructure-expose-scoped-agent-map-planning-tools
…e-expose-scoped-agent-map-planning-tools' into yashnadge/sap-3064-ui-render-the-golden-path-proposal-live

# Conflicts:
#	packages/harness/src/server/index.ts
Base automatically changed from yashnadge/sap-3060-infrastructure-expose-scoped-agent-map-planning-tools to main September 3, 2026 00:19
…-render-the-golden-path-proposal-live

# Conflicts:
#	packages/harness/src/core/planning-session.ts
#	packages/harness/src/core/session-manager.ts
#	packages/harness/src/server/agent-map-mcp-tools.ts
#	packages/harness/src/server/agent-map-mcp-wiring.test.ts
#	packages/harness/src/server/agent-map-mcp.test.ts
#	packages/harness/src/server/index.ts
#	packages/harness/src/shared/agent-map-codec.ts
#	packages/harness/web/src/lib/agent-map.ts
@ynadge
ynadge merged commit 6864bdb into main Sep 3, 2026
11 checks passed
@ynadge
ynadge deleted the yashnadge/sap-3064-ui-render-the-golden-path-proposal-live branch September 3, 2026 04:57
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.

1 participant