Skip to content

feat(harness): migrate graphs to package inventory (SAP-2955) - #731

Closed
ynadge wants to merge 9 commits into
mainfrom
yashnadge/sap-2955-define-the-package-inventory-contract-and-migrate-the-v0
Closed

feat(harness): migrate graphs to package inventory (SAP-2955)#731
ynadge wants to merge 9 commits into
mainfrom
yashnadge/sap-2955-define-the-package-inventory-contract-and-migrate-the-v0

Conversation

@ynadge

@ynadge ynadge commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Primary change type

  • Bug fix
  • Documentation
  • Feature
  • Tests
  • Dependency update
  • Maintenance or refactor

Problem and motivation

The released V0 workspace graph derives identity and navigation from Harness-private registry rows. That does not provide a versioned package contract, makes source identity work part of graph construction, and lets the browser reconstruct path-bearing navigation from mutable workflow state.

This change establishes package inventory as the node/identity contract: which agents exist, their stable identities, and their package-relative locations. Deterministic static graph and package-wide data-flow evidence are explicitly separate concerns and will use independently versioned package-level contracts.

Summary and scope

  • Add strict package inventory protocol 1 to @sapiom/agent, separate from AgentManifest and MANIFEST_PROTOCOL.
  • Keep protocol 1 identity/location-only: it carries canonical or provisional identities, paths, entrypoints, completeness, and working-tree or immutable-bundle versions, but no opaque per-agent relationship payload.
  • Adapt the Harness registry into deterministic public inventory plus private context, with immediate provisional identities and concurrency-bounded background source-name enrichment.
  • Cache enrichment by source fingerprint, invalidate it with graph freshness events, preserve provisional nodes and relationships on failures, and disambiguate duplicate identities deterministically.
  • Commit graph JSON and the private agentKey -> workflowPath resolver atomically under one numeric graph revision.
  • Add the protected GET /api/workspaces/:workspaceKey/system-graph/navigation sidecar and make browser navigation fail closed unless workspace, revision, and lifecycle all match.
  • Preserve current main's direct root-agent focus behavior while retaining revision-matched graph navigation guarantees.
  • Harden refresh, retirement, overlapping request, explicit Retry, and batched-event races; keep mock behavior revision-atomic.
  • Document the HTTP/navigation contract and add Agent and Harness minor changesets.

Intentionally out of scope: package-wide data-flow extraction, static graph evidence contracts, agent input/output extraction, TypeScript Program/TypeChecker analysis, registry-unlinked discovery, bundle inventory emission, backend ingestion, and new graph edge kinds. Those remain for later stacked work.

Related work

Related issue: SAP-2955

Follow-up: SAP-2956

Validation

pnpm --filter @sapiom/agent exec jest --runInBand - 10/10 suites, 174/174 tests passed
pnpm --filter @sapiom/agent typecheck - passed
pnpm --filter @sapiom/agent lint - passed
pnpm --filter @sapiom/agent build - passed

pnpm --filter @sapiom/harness typecheck - passed
pnpm --filter @sapiom/harness lint - passed with one pre-existing warning in untouched src/server/rest.test.ts
pnpm --filter @sapiom/harness build - passed
pnpm --filter @sapiom/harness test:perf - 3/3 files, 10/10 tests passed

focused Harness Playwright mobile + smoke suites - 95/95 passed

regular Harness Vitest suite - 162/164 files and 2678/2680 tests passed
The only two failures are untouched chmod-000 unreadable-directory assertions that cannot reproduce under this capability-enabled sandbox:
- src/core/agent-project-discovery.test.ts
- src/core/workspace-watcher.test.ts

pnpm changeset status --since=origin/main - passed
git diff --check origin/main - passed
static-surface search - no staticSignals, PackageInventoryStaticSignals, "static signal", or "static-signal"

Tests and documentation

Added contract fixtures, provider/cache/invalidation tests, builder/store/router integration tests, strict browser parser and resolver tests, request-race regressions, mock parity coverage, and mounted-view Playwright coverage for batched graph invalidations. Updated packages/harness/docs/workspace-system-graph.md with the protected revision-matched navigation sidecar and background identity lifecycle.

Compatibility and release impact

  • Breaking or externally visible changes: adds a new opt-in @sapiom/agent package-inventory API and a boot-token-protected Harness navigation endpoint. Existing AgentManifest protocol, SystemGraph JSON shape, V0 graph layout, and invocation edge semantics remain unchanged.
  • Package inventory protocol remains version 1 because this API has not shipped.
  • Changesets remain minor for @sapiom/agent and @sapiom/harness; dependent release propagation follows the existing Changesets policy.
  • Current main was merged into this shared stack base without rewriting its published history.

Security

  • I have not included secrets, credentials, private data, or unsanitized logs.
  • This pull request does not publicly disclose a suspected vulnerability.

AI assistance

  • AI assistance was used and reviewed.

Codex implemented and reviewed the contract boundary amendment, mainline conflict resolutions, SDK/Harness behavior, tests, documentation, and final diff. The final branch was validated from an isolated worktree.

Checklist

  • This contribution follows CONTRIBUTING.md.
  • The pull request addresses one focused problem.
  • Relevant tests were added or updated.
  • Relevant build, typecheck, lint, performance, and browser checks were run.
  • Documentation and changesets are included.
  • No hardcoded secrets were added.
  • The final diff was self-reviewed.

ynadge added 2 commits August 28, 2026 09:16
Define the versioned @sapiom/agent inventory contract, move Studio graph identity enrichment behind it, and join path-bearing navigation through an atomic revisioned sidecar.

Closes: SAP-2955
@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #731 (round 1)

1. Unimplemented protocol-1 surface is a permanent public commitment

packages/agent/src/package-inventory.ts:21-24,26-29,43PackageInventoryStaticSignals,
the staticSignals agent field, and version.kind: "bundle" have no producer and no
consumer anywhere in the repo
(grep over packages/, excluding specs). The PR body
lists exactly these — "bundle inventory emission", "static signal producers" — as out of
scope for SAP-2955. They are nonetheless exported from packages/agent/src/index.ts:117-121
and ship in the npm tarball as part of protocol 1.

Failure scenario: a consumer builds against staticSignals — whose payload: JsonValue
and free-form protocol: number assert no semantics and validate nothing — SAP-2956 lands
a different shape, and narrowing it requires a major bump of a package this changeset
bumps as minor. staticSignals also exposes an unbounded recursive jsonValueSchema
(z.lazy over z.record, line 156) to parse input nothing currently emits.

Keep protocol 1 to what is emitted and consumed today; add the fields with the code that
fills them. If they must stay, document their meaning in JSDoc, since the type is the only
documentation an npm consumer reads.

2. Identity enrichment triggers one whole-graph rebuild per agent, undebounced

packages/harness/src/server/index.ts:857-872 and
packages/harness/src/core/system-graph-inventory.ts:770.

onIdentityChange fires once per source root, as each background inspection settles, and
calls systemGraphStore.requestRefresh with no batching window. Every resulting store
transition publishes system-graph.changed (server/index.ts:879-886), and
WorkspaceGraphView reacts by re-fetching the graph and then the navigation sidecar.

Failure scenario: cold-opening a workspace with N registry agents — the normal first open,
since with an empty identity cache every agent starts identity-pending — produces up to
N sequential rebuilds and ~2N invalidation events (a stale and a ready per rebuild).
The card oscillates stale→ready N times and issues two HTTP requests per oscillation, and
each rebuild re-runs listAgents plus the relationship join for every agent.
queueRefresh only coalesces refreshes that arrive while a build is already in flight;
inspections are child processes bounded at concurrency 4 while a rebuild is fast, so most
settle events get a rebuild of their own rather than being absorbed. A short coalescing
window, or one refresh when the inspection queue drains, collapses this to a single
revision bump.

3. The changeset tells an @sapiom/agent consumer nothing

.changeset/bright-graphs-navigate.md — one entry covers both packages in Harness-internal
framing ("migrate Studio dependency graphs to background source identity enrichment with
revision-matched server navigation"). Compiled into packages/agent/CHANGELOG.md and
shipped in the tarball, it never names PACKAGE_INVENTORY_PROTOCOL,
packageInventorySchema, or PackageInventory, so a consumer reading the changelog cannot
tell that a new exported contract exists, what it is for, or that it is opt-in. This is the
one surface that cannot be edited after publish. Split it: a @sapiom/agent entry naming
the new exports, and a @sapiom/harness entry for the graph/navigation change.


Verdict: No confidentiality problems — the changeset, docs/workspace-system-graph.md,
and every new fixture stay generic (acme, @sapiom/*, /Users/demo), and no customer,
internal hostname, or business arrangement appears. The navigation sidecar is correctly
mounted under the boot-token gate (server/index.ts:1503-1586) and paths stay out of
SystemGraph JSON. Approve after 1 and 3; 2 is a follow-up if you would rather not reopen
the lifecycle here.

ynadge commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all three review items in f4047d2:

  1. Kept the bundle/static-signal members because they are explicit SAP-2955 protocol-1 requirements, and added public JSDoc defining immutable bundle semantics plus the versioned, JSON-only, advisory signal envelope and unknown-protocol behavior.
  2. Enrichment results now flush as one sorted source-root batch only after the bounded inspection queue drains. Production requests at most one refresh per containing graph scope, including a seven-agent concurrency regression and an invalidate-before-drain race regression.
  3. Split the release notes into an @sapiom/agent changeset naming the new inventory exports and a separate @sapiom/harness graph/navigation changeset.

Fresh verification: Agent 181/181 + build; Harness graph/inventory focused 128/128 + typecheck/build; lint has 0 errors (one pre-existing untouched warning); terminology, Changesets status, formatting, and diff check pass. Please re-review this head.

@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #731 (round 2, delta since 8ee7837)

Delta is one commit, f4047d2, touching 7 files. Round-1 items 1–3 are all addressed:
JSDoc now defines bundle/static-signal semantics (and the bundle claim at
package-inventory.ts:24-26 is actually enforced at :328-338); enrichment batches;
the changeset is split into a @sapiom/agent entry naming the new exports and a
@sapiom/harness entry. No confidentiality problems in either new changeset.

New finding — batching made identity surfacing all-or-nothing

packages/harness/src/core/system-graph-inventory.ts:781-789. flushIdentityChangesIfIdle
notifies only when activeInspections === 0 && queuedTasks.length === 0, so a batch is
emitted solely on full-queue drain. Inspections run at concurrency 4 as child processes
with a 15s timeout (canvas-manifest-check.ts:35).

Failure scenario: cold-open a workspace with 20 registry agents where one definition hangs
or fails to extract. All 19 already-resolved names sit in the identity cache but nothing
requests a refresh, so every card stays identity-pending in the UI until the slowest
inspection settles — worst case ceil(N/4) × 15s (~75s at N=20), versus names appearing
progressively before this commit. The PR's own new test at
system-graph-inventory.test.ts:394-425 documents exactly this: fast is settled in cache
while the flush waits on slow.

Round 1 offered drain-based flushing as one of two options, so this is the cost of that
choice, not a regression the author introduced blind. The other option — a short coalescing
window (flush pending roots every ~250ms, or on drain, whichever comes first) — collapses
the N-rebuild storm without holding finished identities hostage to the slowest child
process.

Verified non-issues in the delta

Dropping pendingIdentityChanges in invalidateSource/retainSources/clear does not
lose a refresh: the only production caller (server/index.ts:1297) follows invalidation
with systemGraphStore.requestRefresh. The returnbreak change at
system-graph-inventory.ts:742 is correct — the guard re-checks a non-empty queue.


Verdict: Round-1 blockers resolved. Approve; the batching tradeoff above is a
follow-up, not a merge blocker.

ynadge commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the remaining progressive-batching follow-up in 3af0b82.

  • The first settled identity starts a non-resetting 250 ms coalescing window, so completed identities become visible while a slower extraction is still running.
  • A normally drained queue still flushes immediately, preserving the single-refresh cold-open path.
  • Notifications remain sorted/serialized; invalidation, retention, and shutdown cancel stale pending roots/timers.
  • Added a regression with one fast and one blocked extraction proving the fast canonical identity is published while the slow row remains pending, followed by the slow identity in a second batch.

Fresh verification: provider/builder tests 57/57, Harness typecheck, lint (0 errors; one pre-existing warning), production build, Prettier, and git diff --check all pass.

ynadge commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #731 (round 3, delta since f4047d2)

Delta is one commit, 3af0b82, touching system-graph-inventory.ts and its test.
The round-2 follow-up (identities held hostage to the slowest inspection) is fixed: a
non-resetting 250 ms window flushes settled roots while work continues, and a fully
drained queue still flushes immediately (system-graph-inventory.ts:793-807). Timer
cancellation is wired into invalidateSource, retainSources, and clear, and clear()
is called on server close (server/index.ts:2073), so no timer outlives shutdown. The
flushInFlight early-return is safe — the .finally() reschedules. No confidentiality
problems; the delta touches no prose surface and both changesets are unchanged and generic.

New — one cost worth knowing, not a blocker

Each flush calls requestRefresh, and every committed build runs startEnrichment
(system-graph.ts:485system-graph-store.ts:293), which re-enqueues an inspection task
for every source root, each of which re-runs fingerprintWorkflowSources — a recursive
source walk plus a stat per file — before short-circuiting on a fingerprint hit
(system-graph-inventory.ts:774-779). Under drain-batching that whole-workspace walk
happened once per cold open; with the 250 ms window it happens once per flush wave, so a
20-agent workspace at concurrency 4 does roughly 5–6 full re-walks instead of 1. That is
the price of progressive surfacing that round 2 asked for, so it is the accepted tradeoff,
not a mistake — but skipping re-enqueue for roots whose cache entry is already canonical
would remove it entirely.

Nit

  • The 250 ms default is never exercised; both new tests override
    identityChangeCoalesceMs (1 ms and 10 s), so a bad default constant would ship green.

Verdict: All rounds 1–2 findings resolved, nothing new blocking. Approve.

ynadge commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Cleared the remaining test nit in 48b2e2f: the progressive slow-extractor regression now uses the production 250 ms default instead of a 1 ms override. Its fresh run completed the default-window assertion in 304 ms; the full focused provider/builder set remains 57/57, Harness typecheck passes, and the diff is clean.

ynadge commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #731 (round 4, delta since 3af0b82)

Delta is one commit, 48b2e2f: a single deleted line
(identityChangeCoalesceMs: 1) in system-graph-inventory.test.ts:406-410. The
round-3 nit is resolved — the progressive-surfacing regression now runs against the
production IDENTITY_CHANGE_COALESCE_MS = 250 default
(system-graph-inventory.ts:97), so a bad default constant would now fail the suite.
No prose surface, no shipped code, and no changeset moved; nothing new to flag on
confidentiality or public API.

Nit

  • system-graph-inventory.test.ts:413 now waits ~250 ms of real time against
    vi.waitFor's implicit 1000 ms default, in a suite whose own config comment notes
    ~90 test files run in parallel workers under CPU contention. A 4× margin is thin for
    CI; an explicit { timeout: 5000 } on that waitFor (and the one at :425) keeps
    the default-window coverage without the flake risk.

Verdict: All findings from rounds 1–3 resolved; the delta introduces nothing new.
Approve.

ynadge commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Cleared the timing-flake nit in 1d6b985: both waits in the real 250 ms default-window regression now use an explicit 5 s timeout. Fresh focused tests remain 57/57 (the progressive case completed in 305 ms), Harness typecheck and Prettier pass, and the diff is clean.

ynadge commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #731 (round 5, delta since 48b2e2f)

Delta is one commit, 1d6b985, adding { timeout: 5_000 } to the two vi.waitFor
calls in the real-250 ms default-window regression
(packages/harness/src/core/system-graph-inventory.test.ts:413,427) — exactly the
round-4 nit, and nothing else moved: no shipped code, no prose surface, no changeset.

Verdict: All findings from rounds 1–4 resolved; nothing new. Approve.

ynadge commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the full Playwright regression in 4ce82ae: the mock projector correctly gives a root-level provisional agent the checkout-invariant key local:root, but two E2E locators still expected the retired checkout-name-derived key local:rfq-agent. Both locators now assert the protocol key. Fresh verification: all 12 previously failing scenarios pass, the complete mock Playwright suite passes 441/441, targeted Prettier and git diff --check pass.

ynadge commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #731 (round 6, delta since 1d6b985)

Delta is one commit, 4ce82ae, changing two Playwright locators from
system-graph-node-local:rfq-agent to system-graph-node-local:root
(packages/harness/web/e2e/mobile.spec.ts:72, web/e2e/mock-navigation.ts:15).

The new key is the correct one: the rfq mock workflow has definitionSlug: null
(web/src/lib/mock-data.ts:1094) and its path equals the focused scope root, so
projectMockSystemGraphInventory yields inventoryPath === "."
local:root (web/src/lib/api.ts:1384), matching the server-side derivation at
src/core/system-graph-inventory.ts:467-469. No other e2e locator still uses the retired
key; the remaining system-graph-node-* locators target canonical-slug agents and are
unaffected.

No shipped code, prose surface, changeset, or public type moved — nothing new on
confidentiality, semver, or API surface.

Verdict: All findings from rounds 1–5 remain resolved; the delta introduces nothing
new. Approve.

# Conflicts:
#	packages/harness/web/e2e/mobile.spec.ts
#	packages/harness/web/e2e/mock-navigation.ts
Separate cross-agent graph evidence from the package inventory contract so deterministic handoff analysis can define and validate package-level relationships independently.

Refs: SAP-2955

ynadge commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #731 (round 7, delta since 4ce82ae)

Delta is two commits: 8eacb727 (merge of origin/main) and bc3dba8b
(refactor(agent): keep package inventory identity-only).

bc3dba8b removes PackageInventoryStaticSignals, its JsonValue schema, and the
optional staticSignals agent field, plus the matching spec cases. This is a pure
narrowing of an API that has never shipped, so the existing minor changeset stays
correct and no consumer can break. grep finds zero remaining staticSignals /
StaticSignals / static-signal references anywhere in the repo, including the
@sapiom/agent changeset, packages/harness/docs/workspace-system-graph.md, and the
Harness inventory adapter — nothing produces a field the strict schema would now reject.
The replacement JSDoc on PackageInventory is generic; no confidentiality problem.

The merge brings zero changes into packages/harness/src, packages/agent, or
.changeset — main's content lands only in web/, harness-desktop, and mcp. The one
file both sides touched, web/src/lib/use-harness-state.ts, merged into disjoint hunks
(main's openProject/projectToOpen rework vs. this branch's announcement state).

Round 6 correction

Round 6 verified the system-graph-node-local:root locators in web/e2e/mobile.spec.ts
and web/e2e/mock-navigation.ts. Main's #733 deleted those click paths outright — the row
now is the agent, focusRfqAgent never touches a graph node, and no e2e locator uses
that key any more. Not a regression; the round-6 verification is simply moot. The root-key
derivation keeps unit coverage (web/src/lib/api.test.ts:88,
src/core/system-graph-inventory.test.ts:805).

One ask


Verdict: All rounds 1–6 findings resolved or obsoleted; the delta introduces nothing
new. Approve once the post-merge Playwright run is green.

ynadge commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #731 (round 8)

No delta. The PR head (bc3dba8b) is byte-identical to the SHA in the round-7
review marker — no commits, no force-push, no changed files since that review. Nothing
new to report on confidentiality, public API surface, semver/changesets, or code.

Still open from round 7


Verdict: All findings from rounds 1–7 resolved or obsoleted. Approve once the
post-merge mock Playwright run is confirmed green.

ynadge commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #750, which reconstructs SAP-2955 from current main, keeps package inventory identity/location-only, and carries forward the selected fixes from #731 and #741.

I am closing this PR only. Its head branch is intentionally being left intact because #732 still uses that branch as its base; #732 has not been edited, retargeted, rebased, or pushed as part of this work.

@ynadge ynadge closed this Aug 31, 2026
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