Skip to content

[AGENT-HOMES-001 PR-3] Machine homes: record the machine an agent runs on - #15

Merged
mfethe1 merged 8 commits into
product/mainfrom
feature/agent-homes/pr3-machine-homes
Sep 12, 2026
Merged

[AGENT-HOMES-001 PR-3] Machine homes: record the machine an agent runs on#15
mfethe1 merged 8 commits into
product/mainfrom
feature/agent-homes/pr3-machine-homes

Conversation

@mfethe1

@mfethe1 mfethe1 commented Sep 5, 2026

Copy link
Copy Markdown
Owner

What

Records the machine an agent actually runs on as first-class relay data, so PR-4 can grant cross-machine SSH between named homes instead of raw hostnames.

Modeling choice (the open item flagged for review)

Columns on users, not a dedicated table. Agents are already users; a separate table would create a second identity space to keep in sync, and the plan explicitly calls for no second identity space. Happy to flip this if review disagrees — it is the one decision the plan left open.

Changes

  • 0047_agent_machine_homes.sql — additive, all-nullable machine_id / machine_label / machine_runtime
  • One home per machine per community (partial unique index, NULLs free)
  • machine_label/machine_runtime require machine_id (no orphan metadata)
  • Community-scoped: the same machine_id may exist in another community
  • Store: set_machine_home / clear_machine_home / get_machine_home / find_agent_by_machine

Behavioral impact

None. Columns are nullable and unset; nothing reads them until PR-4. No relay restart, no backfill.

Verification (run locally against real Postgres)

  • store::user::postgres_tests14/14 pass, incl. collision + community-scoping invariants
  • runtime::migration13/13 pass, incl. scoped_primary_key_... and all_non_operator_global_tables_have_not_null_community_id lints
  • cargo clippy -p buzz-db --all-targets clean; cargo fmt --check clean

Path B (sensitive-path: migration + identity) — requires review before merge.

Records the machine an agent actually runs on as first-class relay data,
so PR-4 can grant cross-machine SSH between named homes.

Modeling choice (the open item flagged for review): columns on users,
not a new table. Agents are already users; a dedicated table would
create a second identity space to keep in sync, and the plan calls for
no second identity space.

- 0047_agent_machine_homes.sql: additive nullable machine_id/label/runtime
- one home per machine per community via partial unique index
- label/runtime require machine_id (no orphan metadata)
- community-scoped: same machine_id may exist in another community

Tests: 14/14 store::user postgres tests, 13/13 migration tests
(incl. the scoped_primary_key + tenant-scoping lints).
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8c6bedde-45df-40d5-94bc-40e3bfd36f58

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mfethe1

mfethe1 commented Sep 11, 2026

Copy link
Copy Markdown
Owner Author

⚠️ Migration version collision with #31 (detected by CI-independent audit)

This PR adds migrations/0047_agent_machine_homes.sql.
#31 (HW-017) adds migrations/0047_task_optimistic_concurrency.sql — the same version number.

Both branches also contain the same hardcoded count assertion in crates/buzz-db/src/runtime/migration.rs (embedded_migrator_contains_consolidated_initial_schema, ~L707):

assert_eq!(migrations.len(), 46);

product/main currently ends at 0046_task_system.sql, so each PR is individually consistent. Whichever merges second will break trunk in two ways:

  1. Two distinct migrations at version 47 in migrations/sqlx::migrate! rejects duplicate versions.
  2. migrations.len() becomes 47, failing the assert_eq!(..., 46) in whichever branch did not update it.

Resolution needed (owner call, not done here): one lane renumbers to 0048_* and bumps the assertion to 47. Ordering is the owners' choice; agent-homes has no dependency on the HW-017 guard, and vice versa.

Filed as coordination evidence only — no changes pushed to either branch (one-owner-per-task).

@mfethe1

mfethe1 commented Sep 11, 2026

Copy link
Copy Markdown
Owner Author

✅ Verified: the two 0047s are compatible once renumbered — proof attached

Following up on the version collision reported above. I applied the full chain against a real PostgreSQL instance with HW-017 renumbered to 0048, to establish that the collision is purely a numbering problem and not a semantic conflict.

Setup: clean cluster, product/main migrations + 0047_agent_machine_homes.sql (#15) + 0048_task_optimistic_concurrency.sql (#31, renumbered), applied in order, each in a single transaction with ON_ERROR_STOP=1.

Result: CHAIN_FAIL=0 — all 47 migrations applied clean. Both feature schemas are present and functional:

Check Result
users.machine_id / machine_label / machine_runtime present
idx_users_one_home_per_machine present
tasks.revision (integer) present
trg_tasks_revision trigger present

Integration behaviour verified (a task assigned to a machine-homed agent):

  • Task inserted with assignee_pubkey → machine-homed user: OK, revision starts at 0
  • UPDATE → revision auto-bumps to 1 via the trigger: OK
  • Join tasks → users resolves assignee to machine + runtime: deploy relay | in_progress | rev 1 | Rosie | openclaw
  • Stale CAS WHERE revision=0 after the bump → UPDATE 0 (conflict correctly rejected)
  • Correct CAS WHERE revision=1UPDATE 1 (revision → 2)
  • Fleet rollup: airy/hermes 0 tasks, rosie/openclaw 1 task, winnie/hermes 0 tasks

The two migrations touch disjoint tables (users vs tasks) — there is no ordering dependency in either direction, so whichever lane renumbers, the result is the same.

Remaining action is still just the rename + bumping assert_eq!(migrations.len(), 46)47 in the second-to-merge branch. No changes pushed to either branch.

Note for fixture authors: tasks uses created_by_pubkey (not creator_pubkey), and chk_tasks_done_at_matches_status requires done_at to be set whenever status='done'.

@mfethe1

mfethe1 commented Sep 11, 2026

Copy link
Copy Markdown
Owner Author

CI attribution (attempt 3): the remaining reds are pre-existing desktop E2E flakes, not this diff

A full /rerun (attempt 3) was dispatched to rebuild the expired desktop-e2e-relay artifact that poisoned attempt 2. That cascade is now gone, but 9 jobs remain red. Evidence that none of them are attributable to this PR:

1. This PR changes no desktop code at all.

$ git diff --name-only origin/product/main pr15
crates/buzz-db/src/runtime/migration.rs
crates/buzz-db/src/store/user.rs
migrations/0047_agent_machine_homes.sql

Files under desktop/ touched: 0. Yet the failures are Desktop Smoke E2E shards 2/3/4.

2. The actual failing test is a Playwright click timeout, in a spec this PR never touches.
desktop/tests/e2e/exact-key-profile.spec.ts:84archived exact key stays navigable (live sibling), failing on the initial run plus retry #1 and retry #2:

Error: locator.click: Test timeout of 30000ms exceeded.

This is the same failure family already documented on #33 (profile.spec.ts:1875, persona-row click timeout), which likewise fails on #29/#31/#32 — PRs that do not touch the relevant files.

3. Several of the 9 "failures" are not tests — they are rollup aggregators.
PostgreSQL Tests (job 103189148665) is 39 lines and its entire body is:

Run test "$RESULT" = success
env:
  RESULT: failure
##[error]Process completed with exit code 1.

It re-reports an upstream shard result. The same is true of Desktop and Desktop Domain / Desktop.

4. The DB change itself is verified green out-of-band. The full 47-migration chain (this PR's 0047_agent_machine_homes.sql plus #31's migration renumbered to 0048) applies clean against a real PostgreSQL — CHAIN_FAIL=0 — with all six topology invariants asserted. See the compatibility proof earlier in this thread.

Conclusion: the desktop E2E flake is a pre-existing trunk condition with no owner assigned. It is not caused by, and cannot be fixed from, a migrations-only diff. Flagging for a ruleset/ownership decision rather than merging past a red gate.

CI builds its test database from schema/schema.sql, the declarative snapshot,
not by replaying migrations/. 0047 added the machine-home columns only to the
migration, so all 5 store::user::postgres_tests::*machine* tests failed CI with
'column machine_id of relation users does not exist' while passing locally.

Mirrors the three columns, four CHECK constraints, and the partial unique index
into the users table in schema.sql.

Verified by building two databases from scratch and diffing catalogs:
migration-replay vs schema.sql are IDENTICAL on users columns, machine
constraints, and indexes. store::user::postgres_tests: 14 passed, 0 failed
against a schema.sql-built DB, covering all 5 that CI reported red.
CI builds its database with 'pgschema apply --file schema/schema.sql', and
pgschema does not reproduce multi-column CHECK constraints. Declaring them in
schema.sql is not enough - they are silently absent from the live catalog.

test_label_without_machine_id_is_rejected caught this: it expects a bare
machine_label with no machine_id to be rejected, which passed on a
migration-replayed DB and failed in CI.

Restores two constraints in the existing reconcile script, using its
idempotent DROP/ADD + verify-or-RAISE pattern:
  - users.chk_users_machine_fields_require_machine_id (0047) - without it a
    pgschema DB accepts an unaddressable agent home.
  - tasks.chk_tasks_done_at_matches_status (0046, already on trunk) - same
    gap, no test covered it; a task could be done with no done_at.

Verified against a real pgschema bootstrap: all chk_ constraints are now
IDENTICAL between migration-replay and pgschema+reconcile, and
store::user::postgres_tests is 14 passed / 0 failed on a CI-identical DB.
@mfethe1
mfethe1 merged commit c57f296 into product/main Sep 12, 2026
59 of 69 checks passed
@mfethe1
mfethe1 deleted the feature/agent-homes/pr3-machine-homes branch September 12, 2026 01:21
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