Skip to content

test(agent-binding): pin the daemon work-list projection - #1759

Merged
lilyshen0722 merged 1 commit into
mainfrom
kai/task019-assigned-projection-tests
Sep 19, 2026
Merged

lilyshen0722 merged 1 commit into
mainfrom
kai/task019-assigned-projection-tests

Conversation

@lilyshen0722

@lilyshen0722 lilyshen0722 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Tests only. The second half of TASK-019: #1756 pins the CLI's consumption of the server's installation list; this pins the server's projection of it — the surface the daemon actually reads and writes into its seat record.

The gap (measured on ac544d5c)

GET /api/agent-binding/assigned (backend/routes/agentBinding.ts:388) builds one row per identity out of the caller owner's active installations:

const installs = await AgentInstallation.find({ installedBy: machine.ownerUserId, status: 'active' })
  .select('agentName instanceId podId config').lean();          // :394
...
if (install.podId) entry.podIds.push(String(install.podId));     // :417
if (!entry.runtime && config.runtime) entry.runtime = config.runtime;       // :418
if (!entry.environment && config.environment) entry.environment = ...;      // :419

Two properties of that projection carried no test:

  1. The owner filter is load-bearing after bind time. The sole-installer clause (ownsAgent, :262) is checked at adopt: a second installer of the same (agentName, instanceId) is refused with another_installer. That check does not exist at read time — the filter on :394 is the only thing holding once a second installer appears afterwards. If it leaks, a stranger's runtime, MCP servers and pod reach this daemon's seat record.
  2. One row per identity. An identity installed by its owner in several pods (the taxonomy's one-install-fans-out) is one daemon row with podIds unioned — the daemon mints one seat per identity, so a duplicate row would mean the same agent in the work list twice.

What is pinned

  • ignores another installer's installation of the same identity: bind on machine-a, then have a second user install the same identity in their own pod with runtime.model: 'stranger-model' and an stranger-mcp server. The response is still one row, carrying the owner's runtime and only the owner's pod; the serialized row contains neither the stranger's model, nor their MCP server, nor their podId.
  • projects one row per identity across its owner's installations, unioning podIds: a second owner installation in a second pod yields one row whose podIds holds both.

Mutation proof

Both tests discriminate rather than assert:

  • dropping installedBy: machine.ownerUserId from :394 → exactly the first test fails (19 pass, 1 fail);
  • overwriting entry.podIds instead of pushing → exactly the second test fails (19 pass, 1 fail).

Deliberately NOT pinned: which installation wins

find() at :394 has no .sort() anywhere in the file, and :418/:419 take the first non-empty value per field independently. So when one identity has installations carrying different runtimes or environments: which one reaches the daemon is document order (an accident that holds until an index or a write reorders it), and runtime and environment can come from different installations. That is not a precedence rule, and choosing one (newest? the installation whose pod the daemon is bound to? the oldest, stabilising today's first-wins?) is a behaviour change for live seats — so it is recorded on the board row rather than settled in a test that would enshrine whichever order in-memory Mongo happened to return. The second test asserts the part that is a contract: the projection is one of the declared values, never a merge or an invention.

Scope

One test file, no source change. No version bump: the package-version guard watches cli/src and commonly-mcp/src only, and the backend is not a published package. __tests__/unit/routes/agentBinding{,.phase2}.test.js → 29 passed with the new cases.


Retracted: the (i) behaviour change is NOT in this PR

An earlier head of this branch (59d99388) folded the TASK-019 ruling (option i)
into this PR. Wren's sequencing ruling (pod message, 10:14Z) says otherwise:
#1759 merges as cleared at 36bfadfc, tests-only; the behaviour change is a
separate PR cut from main AFTER this one merges — not folded, not stacked.

The head is back at 36bfadfc (force-pushed, one file, one commit), so the
reviewed and cleared content is exactly what merges. The fold is preserved as a
patch and will be re-cut from updated main once this lands: it orders the find
(_id ascending), makes ONE installation supply both halves, leaves a silent
half null, and keeps podIds a union. The two witnesses added there are
recorded with it.

Nothing in this PR changes: it pins the two contracts without pinning the
ordering accident.

@lilyshen0722 lilyshen0722 changed the title test(agent-binding): pin the daemon work-list projection fix(agent-binding): pin the daemon work-list projection, and take the runtime+environment pair from the oldest declaring installation (TASK-019 ruling i) Sep 19, 2026
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Fold-in note for whoever merges, and the one thing in here that is a tripwire rather than a proof.

The pod ruling on TASK-019 (option i) is now implemented on this branch, not only witnessed: the find is ordered (_id ascending) and one installation supplies BOTH halves, with podIds still unioned and a silent half left empty. Before this, the two halves came from whichever documents the unordered find returned first, so runtime and environment could be sourced from different installations. That is why the second test's comment changed — it previously declined to pin the order because there was no rule to pin.

Two mutations were run against the final tree: swapping the selection back to main's per-field fill reddens the silent-half test only; making it last-declaring-wins reddens both that test and the union test. Removing the .sort({ _id: 1 }) clause reddens nothing — mongodb-memory-server hands back insertion order, so the clause is a tripwire against unspecified natural order and no test at this tier can witness it. Flagging that explicitly rather than letting the green suite imply the ordering is proven.

Head 59d99388 (was 36bfadfc); the earlier clearance does not transfer.

@samxu01
samxu01 force-pushed the kai/task019-assigned-projection-tests branch from 59d9938 to 36bfadf Compare September 19, 2026 10:16
@lilyshen0722 lilyshen0722 changed the title fix(agent-binding): pin the daemon work-list projection, and take the runtime+environment pair from the oldest declaring installation (TASK-019 ruling i) test(agent-binding): pin the daemon work-list projection Sep 19, 2026
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Retraction, and it is mine to state: I folded the TASK-019 (i) behaviour change into this branch before wren's sequencing ruling landed ("#1759 merges as cleared at 36bfadf (tests-only pin); the behaviour change is a separate PR cut from main AFTER #1759 merges — not folded into #1759, not stacked"). His ruling is the one that counts, and it arrived while I was pushing, so what looked like a disagreement is a race.

The head is back at 36bfadfc (force-pushed) — one commit, one file, the exact content that was cleared. The fold is preserved as a patch and will be re-cut from updated main after this merges, carrying both witnesses (the silent-half test and the tightened union assertion) with the ordering clause.

So: nothing to re-review here; the earlier clearance re-attaches to a head that is byte-identical to the one it was given for. The re-gate request I sent for 59d99388 is withdrawn.

One row per identity across its owner's installations (podIds unioned), and another installer's installation of the same identity contributes no runtime, no MCP servers and no pod to this daemon's work list. The sole-installer clause 403s at adopt time; a second installer can appear afterwards, and the installedBy filter on /assigned is what holds then.
@lilyshen0722
lilyshen0722 force-pushed the kai/task019-assigned-projection-tests branch from 36bfadf to 6a57988 Compare September 19, 2026 15:43
@lilyshen0722
lilyshen0722 merged commit 8cb5880 into main Sep 19, 2026
14 checks passed
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