Skip to content

fix(harness): reset legacy maps and initialize existing projects - #844

Open
ynadge wants to merge 7 commits into
review/agent-map-16-remove-empty-project-actionfrom
review/agent-map-17-legacy-initialization
Open

fix(harness): reset legacy maps and initialize existing projects#844
ynadge wants to merge 7 commits into
review/agent-map-16-remove-empty-project-actionfrom
review/agent-map-17-legacy-initialization

Conversation

@ynadge

@ynadge ynadge commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Primary change type

  • Bug fix

Problem and motivation

Users opening a released Studio profile can have legacy format-1 maps or existing agents with no map. Previously, those projects remained in a storage error or empty state. Reset legacy format 1 deliberately and initialize genuinely absent maps once from static contract evidence, while protecting every authored format-2 map from automatic edits.

Summary and scope

  • Reset only records whose outer storageSchemaVersion is numeric 1, under the existing project lock, with a durable reset journal. Apply the same policy during shared desktop/CLI startup and late store access. Preserve agent files, project identity, sessions, and conversation history.
  • Separately recognize four exact unused historical wrapped-format-2 shapes. Require the initial revision, identical valid timestamps, null pointers, and every map/planning/history collection empty. Reread under the project lock, durably preserve an exact source backup, and atomically convert to the current empty aggregate before initialization eligibility. Current format 2, authored or cleared historical maps, malformed data, and unknown variants never enter this conversion.
  • Scan each available restored project root through bounded static discovery. Defer scheduling until startup scans finish, publish only status changes, and exclude the actual map metadata subtree from workspace events to avoid self-triggered discovery invalidation. Queue one project-owned initialization attempt only after complete discovery and a confirmed missing or unused map container. Invalid state, read failures, unsupported formats, authored-empty maps, and accepted operation history do not count as absence.
  • Run isolated structured inference through TaskManager using the most recent available project provider or the host default. Support both Claude Code and Codex with native configured default models, no coding/MCP tools or Studio write credentials, no source execution, no public task payloads, and no session tab. Default to two concurrent tasks and a three-minute timeout; interrupted and failed attempts need explicit retry.
  • Validate all agent identities and contract references using the existing map semantics. Publish the complete initial graph through a host-authorized, create-only transaction that rechecks ownership and history under the map lock. An intervening coding-session map wins; completed initialization cannot write again. Coordinate this ownership with ordinary Plan Agents bootstrap.
  • Add authenticated project status/retry endpoints and bounded change events. Show compact generating/failure states, keep storage errors separate, and pack connected components, disconnected nodes, cycles, and long chains into balanced layouts. Preserve selection and manual viewport control.

How this increment fits

This implements the maintainer-approved legacy-reset and initial-map process above the previous sidebar fix. The complete change is also on fix/studio-onboarding-followups, the existing local testing branch.

Stack and review boundary

  • Base: #836 — remove empty-project agent creation button, branch review/agent-map-16-remove-empty-project-action.
  • Predecessor-relative review: 52 files, 4,806 additions, 167 deletions, including tests, documentation, and a Changeset.
  • Complete testing branch: fix/studio-onboarding-followups, commit bdfadd9483b3753990647236373bdc7748260905.
  • Review head: ba55870f2d0795da4cf29872e56dbffdda32e9b9. Both commits have the identical source tree. Earlier PRs remain unchanged and unmerged.

Approved startup behavior

Automatic initialization is enabled for both desktop and CLI, exactly as specified in the maintainer-approved plan. This requested behavior change supersedes the package checklist's generic default of leaving CLI setup unchanged. Generation remains bounded to one automatic attempt per eligible project; failure does not switch providers or trigger automatic retries.

Related work

Maintainer-approved migration and startup behavior following #836 and the original Agent Map stack. The approved compatibility extension handles provably unused historical wrapped-format-2 containers. Populated or uncertain historical wrappers remain load errors requiring a separate data-preserving migration.

Validation

pnpm build — passed
pnpm typecheck — passed
pnpm lint — passed
pnpm test — passed across the repository
  harness: 235 unit files / 3,847 tests; 3 performance files / 10 tests
  harness-desktop: 21 files / 172 tests
Focused Playwright on the unchanged UI: agent-map-initialization + project-map-navigation — 30 previously passed, zero retries
Current compatibility, initialization/store, and source inspection tests — 131 passed
Native Claude Code and Codex, controlled local provider responses — 10 cases passed
Real Mac Claude Code and Codex inference on copied release contracts — passed
Current Mac source: harness build; desktop build and pack — passed
Packaged Mac smoke — 14 passed; 1 Windows-only check skipped
Copied desktop startup → conversion → real generation → six-agent browser map → restart — passed
Authenticated ordinary-session MCP edit → browser rendering → restart → initialization retry — passed
Git diff whitespace check and canonical/review source-tree comparison — passed

Root gates cover the final application source. Browser checks use the built mock UI, an isolated preview, and one Chrome worker. The restored-root regression fails against the previous startup code and passes with the fix. Native process tests cover bounded output, tool restrictions, default-model preservation, authentication failures, cancellation, timeout, and cleanup. Live provider checks also verify ordinary coding-session edits and reopening without regeneration.

Tests and documentation

Added reset safety, eligibility, ownership/race, interruption/retry, evidence validation, native credential isolation, and startup wiring tests. Follow-up tests prove damaged reset markers cannot block healthy format 2 and unnamed native watch events fall back to polling without losing real source edits. Added browser coverage for generation states, separate load errors, cross-host status observation, 1/10/50/100 agents, chains, fan-out, cycles, disconnected components, later edges, selection, and viewport behavior.

The format-1 reset journey verified five legacy records and preserved eleven format-2 files byte-for-byte during reset. The current compatibility journey used a fresh desktop-profile copy with fourteen map files: six exact unused old-format-2 containers were converted with byte-identical backups, and all eight other files stayed unchanged. The final copy preserves dependency symlinks and nested repository markers, matching the original static-source boundaries. Three eligible projects completed real Claude Code inference; the selected six-agent project rendered all agent identities and evidence-backed supporting nodes. Two projects correctly failed before inference because an agent has no supported source files in both the original and copied directories; one project remained idle. No generation session tabs were created, all 83 existing session IDs were retained, and all copied source files stayed unchanged.

A normal authenticated project-session MCP edit appeared in the browser and survived restart; this test used an idle PTY rather than a native coding conversation. Restart and explicit initialization retry did not overwrite the generated or edited map and caused no repeated inference. Tests additionally cover all four old container variants, exact backup bytes, backup failure, interrupted persistence, lock contention, concurrent hosts, and coding sessions writing before dispatch or commit. The regression reproduced the original malformed-state failure on all four variants before the fix.

Live verification found a linked node_modules directory incorrectly marking static evidence incomplete. The source walker now excludes links at already-ignored dependency/build/metadata boundaries while keeping unknown directories and source-file links opaque. Six new regressions fail before this fix; the positive run and full repository gates pass afterward.

The final actual desktop-profile localhost check passed: all six agents in the affected project rendered, seven previously authored maps and 445 checked source files stayed byte-for-byte unchanged, and the two link-related failed attempts completed after explicit authenticated retries. Two separate projects still lack supported contract sources; one populated historical wrapper remains protected for a separate migration.

Updated the harness README, added a minor Changeset, and extended packaged smoke coverage to import the unpacked initialization worker in a real Node-mode child.

UI examples

Captured from the built UI with synthetic 100-agent graphs. Both examples retain all nodes and provide Fit/zoom controls.

100-agent chain

100 agents in multiple components

Compatibility and release impact

  • Externally visible changes: legacy format 1 is reset; existing projects can generate their first map in the background. Provably unused historical format-2 wrappers are backed up and converted separately; valid unused format-2 containers may receive their first map. Existing authored format 2, the storage schema, and ordinary coding-session map tools remain compatible.
  • Native CLI credential helpers that require executing user scripts are excluded from background inference. Provider authentication or execution failure remains an explicit retryable failure; there is no silent provider fallback.
  • Changeset: .changeset/initialize-existing-project-agent-maps.md (@sapiom/harness: minor).

Security

  • No secrets, credentials, private user data, raw contracts, model output, or unsanitized logs are included.
  • This PR does not publicly disclose a suspected vulnerability.

AI assistance

  • Codex implemented the change, inspected the final diff, obtained an independent review, and ran the checks above. Live checks found and fixed missing relationship-semantic instructions, restored-project startup discovery, and active-session watcher feedback; review found and fixed cancellation during final authorization.

Checklist

  • Read CONTRIBUTING.md; the migration behavior was explicitly approved by the maintainer.
  • Description reflects the actual predecessor-relative diff.
  • Tests cover reset safety, publication authority, provider execution, rendering, and desktop startup.
  • Required build, typecheck, lint, and test commands passed.
  • User-facing behavior and release impact are documented.
  • The testing and review branches have identical source trees.
  • No PR was merged.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Review — PR #844 (round 1)

No confidentiality issues: the changeset, README section, comments and fixtures stay on
Sapiom + the public Claude Code/Codex vocabulary. company.gateway / acme-app are
generic placeholders.

1. patch understates a feature-and-behavior change on a published package

.changeset/initialize-existing-project-agent-maps.md@sapiom/harness is public
(publishConfig.access: public, 0.14.0) and src/index.ts does
export * from "./shared/types.js", so this PR widens the published type surface:
StructuredInferenceOptions, SpawnSpec.stdin, HarnessAdapter.supportsCodingTasks,
two AnalyticsEventType members, and a new BusMessage union member
(agent-map.initialization.changed). A consumer with an exhaustive switch +
never check over BusMessage stops compiling on what npm reports as a patch bump.
On top of that the release deletes user workspace.json files at startup and adds two
REST endpoints.

This should be minor, and the changeset body should state the two externally visible
behaviors the PR description already admits to: legacy format-1 maps are deleted, and
existing projects start generating maps in the background.

Related: AgentMapInitializationStatus is now referenced by the public BusMessage
type, but exports only ships "." — consumers can see the type and cannot name it.

2. Automatic, billed provider inference on startup with no way to turn it off

packages/harness/src/server/index.ts (scheduleExistingMaps / initializationDiscovery)
schedules up to two concurrent Claude Code or Codex runs, 3-minute timeout each, over
every restored project that has agents and no map — unconditionally, on both hosts,
including npx @sapiom/harness. A user who updates and opens Studio with 20 legacy
projects gets 20 provider invocations charged to their own subscription, with no consent
step, no setting, and no HarnessServerOptions field.

packages/harness/CLAUDE.md ("Checklist: adding setup-time behavior") requires exactly
that: "Value arrives as a HarnessServerOptions field, with a default that keeps the
CLI unchanged."
Add the toggle (persisted, so GET /api/state and the next launch agree
— rule 1 of the same file), or gate generation on an explicit user action.

3. A corrupt legacy-reset.json permanently bricks a healthy project's map

packages/harness/src/core/agent-map-workspace-store.ts:238 — any non-ENOENT failure
reading or parsing the marker throws storageError(). resetLegacyLocked is called at
the top of readDisk (:296), so that throw sits on every locked() path: reads,
transactions, MCP map tools, inspectInitialization, bootstrap.

Failure scenario: writeSidecar is interrupted (disk full, SIGKILL, or an
externally-truncated file) leaving legacy-reset.json non-JSON. workspace.json is
intact valid format 2, but from then on every map operation for that project throws
storage_unavailable, and the SPA's "Reload map" button just re-throws — there is no
recovery path short of hand-deleting the file. Note the asymmetry: :218 deliberately
tolerates a SyntaxError on the primary workspace file ("preserved for the map reader
to classify") while :238 will not tolerate one on the sidecar.

Fix: treat a malformed/unreadable marker as absent, exactly like ENOENT. Separately,
hoisting resetLegacyLocked out of readDisk into the startup pass would also stop
every locked operation paying a second full readFile + JSON.parse of workspace.json
plus a marker read — which the new 2s status poll multiplies while generation runs.

4. HYGIENE (advisory) — a second recipe for refreshing initialization status

packages/harness/web/src/lib/use-agent-map-entry.ts — the polling effect re-implements
the fetch → setInitialization → setInitializationLoadError(null) → agentMapLoader.invalidate → loadWorkspace(projectId, true) sequence that refresh/show in the effect directly
above already owns, with its own revision guard. Every later change to completion handling
has to be made twice. Concrete step: have the interval call
refreshInitializationRef.current?.(). That also removes the second issue here — the
effect lists initialization in its deps while replacing that object on every tick, so
the 2s interval is torn down and recreated on each response.


Verdict: request changes — bump the changeset to minor with the reset and background
generation spelled out, gate automatic inference behind a HarnessServerOptions option,
and stop a bad marker file from bricking the project map.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Review — PR #844 (round 2, follow-up)

Delta since 9d2b160b: one commit, 9b2565fb ("prevent map initialization watcher
feedback") — watcher ignoredEventRoots, status-publish de-duplication, and gating
scheduleMapInitializations on completed discovery, plus tests. No new prose surfaces,
no confidentiality issues in the delta.

Earlier findings NOT fixed by this push

  1. Changeset is still patch (.changeset/initialize-existing-project-agent-maps.md)
    for a release that adds a BusMessage union member, new exported types, deletes user
    workspace.json files at startup and adds two REST endpoints. Still needs minor with
    the two externally visible behaviors spelled out.
  2. No opt-out for automatic billed inference. startServer still schedules provider
    runs over every eligible restored project with no HarnessServerOptions field, contrary
    to the packages/harness/CLAUDE.md setup-time-behavior checklist. (The push actually
    makes this stricter: scheduleMapInitializations is now armed only after discovery, so
    scheduling is more centralized, not more optional.)
  3. A malformed legacy-reset.json still bricks the project map.
    agent-map-workspace-store.ts:238 still does if (code !== "ENOENT") throw storageError(),
    on a path reached from every locked() operation. Treat an unparseable marker as absent.
  4. HYGIENE (advisory) — web/src/lib/use-agent-map-entry.ts untouched: the poll effect
    still duplicates refresh and still re-creates its 2s interval on every response.

New finding

  1. The feedback fix has a hole on null-filename watch events.
    workspace-watch-broker.ts:644 returns potential(null) + scheduleSourceChange(null)
    before the new ignoredEventRoots check at :651. When fs.watch delivers a null
    filename (platform/backpressure dependent), a lock or journal write under
    statePaths.agentMap still invalidates discovery and re-triggers eligibility checks —
    i.e. the exact loop this commit removes for the named-file case. The de-dupe in
    agent-map-initialization.ts hides the resulting event storm from the UI but not the
    repeated lock/readdir work. Neither new test covers the null filename path.

Verdict: request changes — the three round-1 blockers (changeset level, missing opt-out,
marker-file brick) are all still open, and the watcher fix leaves the null-filename path.

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