feat(agent): consume Gram-Device-Environment on getPlugins - #6022
Merged
Conversation
Migration only, no application code — the table is unused until the follow-up wires the agent poll to it. Records the agent heartbeat from a box that is not somebody's laptop: a cloud coding sandbox, a container, a shared self-hosted server. The device agent declares which through a new Gram-Device-Environment header; absent means laptop, so every deployed agent keeps writing to device_agent_syncs exactly as it does today. Keeping those rows OUT of device_agent_syncs is the point. The coverage join falls back to matching an MDM device's assigned-user email against that table, so a cloud session polling under a real person's address would mark that person's laptop agent_active whether or not the laptop runs the agent. A false coverage claim is worse than an absent one, and the shared-identity setup our docs describe for cloud environments makes it easy to hit by accident. A separate table rather than an environment column, because widening device_agent_syncs would mean changing its (organization_id, email) unique key, which the upsert infers by name. Migrations ship ahead of code, so the already- deployed server would spend that window issuing ON CONFLICT against an index that no longer exists. Keyed on (organization_id, LOWER(email), environment): one identity legally polls from several kinds of box, and those are different facts that must not overwrite each other. The expression index matches how every reader compares email, for the same reason the sibling device table uses one. Purely additive — a new table, no lock taken on anything existing, and no PG305/PG306 scan.
Stacked on the migration that adds device_agent_environment_syncs.
The device agent now declares what kind of box it runs on. A non-laptop
heartbeat records into the new table INSTEAD of device_agent_syncs, which is
the whole point rather than a detail.
Coverage falls back to matching an MDM device's assigned-user email against
device_agent_syncs:
WHEN das.last_seen_at >= @active_cutoff::timestamptz THEN 'agent_active'
That fallback runs in both matching modes, so a cloud session polling under a
real person's address marks their MDM laptop agent_active whether or not the
laptop is running the agent. Cloud environments enroll with a single shared
identity and using a real person's address is the natural first thing to try,
so this is easy to hit by accident — and a false coverage claim is worse than
an absent one, because nothing prompts anyone to look.
The attribute is deliberately not a Goa Enum, and unrecognized values degrade
to laptop rather than erroring. Rejecting the poll would stop that device
syncing plugins at all, which is an outage caused by an attribution hint. A
newer agent inventing a kind this server has not heard of keeps working, and
lands where it lands today.
Absent, empty, laptop, and unrecognized all behave identically to today, so
every deployed agent's poll is unchanged.
Note for whoever picks up the aggregation work: this fixes the false positive
and records cloud usage, but nothing reads the new table yet. The original
ticket described a different problem — cloud runs inflating coverage
denominators — which cannot happen: the denominator counts mdm_devices, and a
box with no serial writes no device row at all.
Contributor
🦋 Changeset detectedLatest commit: 223ca73 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Running ultrareview automatically — This changes a production agent API and reroutes heartbeats between persistence tables, affecting coverage truth across organizations; subtle normalization, compatibility, or upsert errors could silently misreport device status at scale.. I'll post findings when complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Ultrareview completed in 7m 18s
All reported issues were addressed across 15 files
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Matches the device-agent side (speakeasy-api/device-agent#190). "laptop" was wrong the day it was written — the same value covers a desktop, and would have to cover a phone. Naming a catch-all after one of its instances guarantees the name expires. "endpoint" is the MDM and compliance vocabulary for an end-user device regardless of form factor. Semantics are unchanged: still the default, still the value the agent never sends, still what an unrecognized value degrades to. Nothing has shipped with the old value, so this costs nothing now and would have been a wire-compatibility problem across two repos later.
sqlc derives models.go from the schema, so the new table's struct belongs with the migration that creates it. It was on the follow-up PR instead, which left this one failing the build-lint tree-clean check.
…o-1005-consume-device-environment
The goa design change adds a header to getPlugins, which flows into the Gram-Internal OpenAPI output and the generated dashboard SDK. CI checks the tree is clean after regenerating, so these belong in the same PR as the design change rather than arriving as drift later.
… too The environment split closed the email-matched path — a cloud session polling under a real person's address no longer marks their laptop covered. It left the serial-matched path open: a `server` or `ephemeral` box that reports a serial still wrote device_agent_device_syncs, which backs DEVICE-level coverage. Same hole, different join. A shared server or an ephemeral box is not a managed endpoint, and nothing about it belongs in a per-device count; it is recorded as an environment instead. The endpoint path is unchanged and tested to stay that way. Not addressed, deliberately: the ticket also asks getPlugins to persist Gram-Device-Instance "if the agent sends one". The agent does not send one, and the device-agent side records why — Cursor Cloud exposes its session id only inside the agent's own process, so the setup script that runs the reconcile cannot read it, and a header populated on one platform and empty on the other is worse than absent because it looks like data. Adding a column for a header no client sends would be speculative schema.
daviddanialy
reviewed
Sep 3, 2026
daviddanialy
approved these changes
Sep 3, 2026
Its timestamp predates 20260903163542 on main, and atlas.sum conflicts. Per the repo's rule, migrations are regenerated rather than hand-merged or renamed.
…ment-syncs-migration
Regenerated on top of main after 20260903163542 landed. The previous file was timestamped 20260903161819, which is earlier, so it was both out of order and conflicting in atlas.sum. Deleted and re-run through `mise db:diff` rather than renamed or hand-merged, per the repo's rule. The generated DDL is byte-identical to the version this replaces — only the timestamp and the atlas.sum position changed.
…o-1005-consume-device-environment
Answering the review question directly: there was no case for environmentEndpoint, and the constant was declared but never read, so a reader could not tell whether endpoint was handled deliberately or forgotten. That is a fair thing to trip over. The function now returns one of the three constants for every input and never "", with an explicit endpoint case so the closed set is visibly exhaustive. Both call sites compare against the constant by name rather than against an empty sentinel, which also removes the conflation of "explicitly an endpoint" with "nothing here". Behavior is unchanged. Three inputs still collapse onto endpoint — an explicit "endpoint", an absent header from any agent predating the field, and an unrecognized kind — because they mean the same thing to this server: an ordinary device, recorded the way it always was. An internal test pins totality, since the property is what makes the call-site comparisons safe and the external test package cannot reach the function.
Base automatically changed from
brad/dno-1005-environment-syncs-migration
to
main
September 3, 2026 19:21
…-device-environment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #6021, which adds the table. Pairs with the agent side in speakeasy-api/device-agent#190.
The bug this fixes
Coverage falls back to matching an MDM device's assigned-user email against
device_agent_syncs:That fallback runs in both matching modes. So a cloud session polling under a real person's address marks their MDM laptop
agent_activewhether or not the laptop is running the agent.It's easy to hit by accident: cloud environments enroll with one shared identity, and using a real person's address is the natural first thing to try. And a false coverage claim is worse than an absent one, because nothing prompts anyone to look at it.
What changed
getPluginsaccepts aGram-Device-Environmentheader. A non-laptop heartbeat records intodevice_agent_environment_syncsinstead ofdevice_agent_syncs— keeping those rows out of the table coverage reads is the whole point, not a detail.Absent, empty,
laptop, and unrecognized values all behave exactly as today, so every deployed agent's poll is unchanged.Two deliberate choices worth a look
The attribute is not a Goa
Enum, and an unrecognized value degrades to laptop rather than erroring. Goa rejects an out-of-set value with a 400, and a rejected poll means that device syncs no plugins at all — an outage caused by an attribution hint. A newer agent inventing a kind this server hasn't heard of should keep working; the worst case is its heartbeat counts as a laptop, which is where it lands today anyway.The key is
(organization_id, LOWER(email), environment). One identity legitimately polls from more than one kind of box — the same address may run a laptop agent and be the shared identity a fleet of sandboxes enrolls with. Those are different facts and must not overwrite each other. There's a test for it.Correction to the original ticket
The ticket described a different problem: cloud runs inflating coverage denominators with phantom devices. That can't happen. The denominator counts
mdm_devices, and a box with no serial never reachesUpsertDeviceAgentDeviceSync, so it writes no device row at all. I went looking for it and found the false positive above instead, which points the other way — not inflated denominators, but falsely satisfied coverage.I've written that up on the Linear issue too.
Not in scope
Nothing reads the new table yet. Surfacing cloud usage in the dashboard, and any bucketed aggregation, is follow-up work — this PR is the persistence plus the correctness fix.
Checks
go build ./...andgo vetclean.internal/agentandinternal/deviceintegrationstest packages pass, including three new tests covering the routing, the unchanged-laptop-behavior matrix, and coexistence of environments for one identity.Summary by cubic
Adds the optional
Gram-Device-Environmentheader togetPluginsso cloud and shared-server heartbeats no longer count as evidence that a person's own endpoint is covered. Previously, a sandbox or server polling under a real email could mark that person's MDM deviceagent_activethrough the email-matched or serial-matched fallback; nowephemeralandserverheartbeats are recorded indevice_agent_environment_syncsinstead, and endpoint polls behave exactly as before.endpoint, and unrecognized values keep writing todevice_agent_syncsanddevice_agent_device_syncsas today; unrecognized values degrade toendpointrather than rejecting the poll.ephemeralandserverheartbeats never write to either coverage-read table, even when a serial is present.Gram-Device-Instanceis intentionally not persisted: the agent does not send it, and a column for a header no client sends would be speculative.Written for commit 223ca73. Summary will update on new commits.