feat(openframe): stamp team_id on CDC-captured tables for shared-DB Debezium - #77
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds an OpenFrame migration that introduces nullable 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/datastore/mysql/policies.go (1)
958-970: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSentinel comment is missing its
>>>marker.Line 963's
// OPENFRAME(mysql-multitenancy): ...comment lacks the>>>prefix used everywhere else in this file (e.g., lines 910, 2269), and there's no matching<<<close. This breaks the grep-based fork-audit convention for locating OpenFrame edits during upstream merges.As per coding guidelines, "Fork edits in shared upstream files must be wrapped in sentinel comments with the format
// >>> OPENFRAME(<slug>): <why> — openframe/docs/<doc>.mdfollowed by fork code, then// <<< OPENFRAME(<slug>)."🏷️ Proposed fix
query := fmt.Sprintf( // INSERT IGNORE skips rows whose policy_id no longer exists (policy deleted // after query was distributed but before results arrived). - // OPENFRAME(mysql-multitenancy): column list / dup-update carry team_id when the - // request is team-pinned (see membershipCols above). + // >>> OPENFRAME(mysql-multitenancy): column list / dup-update carry team_id when the + // request is team-pinned (see membershipCols above) — openframe/docs/mysql-multitenancy-feature.md. `INSERT IGNORE INTO policy_membership (%s) VALUES %s ON DUPLICATE KEY UPDATE %s`, membershipCols, strings.Join(bindvars, ","), membershipOnDup, + // <<< OPENFRAME(mysql-multitenancy) )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/datastore/mysql/policies.go` around lines 958 - 970, Update the OPENFRAME comment near the policy_membership INSERT in the transaction callback to use the required `>>> OPENFRAME(mysql-multitenancy)` opening sentinel with its rationale and documentation reference, and add the matching `<<< OPENFRAME(mysql-multitenancy)` closing sentinel around the fork-specific SQL changes. Preserve the existing query behavior and keep the sentinel scope limited to the OpenFrame modification.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/activity/internal/mysql/new_activity_test.go`:
- Around line 297-303: Update the activity_past query in the test around actID
and actTeam to filter by the specific request pin or other identifiers
established by the test, rather than only activity_type = 'ran_script'. Ensure
the lookup uniquely selects the activity created by this test and does not
depend on arbitrary row ordering or table truncation.
---
Nitpick comments:
In `@server/datastore/mysql/policies.go`:
- Around line 958-970: Update the OPENFRAME comment near the policy_membership
INSERT in the transaction callback to use the required `>>>
OPENFRAME(mysql-multitenancy)` opening sentinel with its rationale and
documentation reference, and add the matching `<<<
OPENFRAME(mysql-multitenancy)` closing sentinel around the fork-specific SQL
changes. Preserve the existing query behavior and keep the sentinel scope
limited to the OpenFrame modification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e33a1c2-1586-45ca-856f-1363995fa6a3
⛔ Files ignored due to path filters (2)
openframe/docs/fork-file-manifest.mdis excluded by!**/*.mdopenframe/docs/mysql-multitenancy-feature.mdis excluded by!**/*.md
📒 Files selected for processing (6)
server/activity/internal/mysql/new_activity.goserver/activity/internal/mysql/new_activity_test.goserver/datastore/mysql/migrations/openframe/20260722000001_AddTeamIdToCdcTables.goserver/datastore/mysql/migrations_openframe_test.goserver/datastore/mysql/policies.goserver/datastore/mysql/query_results.go
oleksandrd-flamingo
enabled auto-merge
July 24, 2026 15:55
oleksandrd-flamingo
disabled auto-merge
July 24, 2026 15:55
…ebezium
Under Fleet shared-DB multi-tenancy one MySQL serves every tenant, so a CDC
record must carry its own tenant discriminator: the Debezium connector's SMTs
are stateless and cannot join host_id -> hosts.team_id, and activity_past rows
have no host reference at all. This stamps a team_id column on the four
CDC-captured tables so the shared-plane stream consumer can resolve the tenant
per event.
- Migration 20260722000001_AddTeamIdToCdcTables (openframe goose pipeline):
nullable team_id on activity_past, activity_host_past, query_results,
policy_membership. No index, no FK (a team deletion must not rewrite/cascade
over historical CDC rows). Idempotent via columnExists guards.
- Write-path stamping, all wrapped in OPENFRAME(mysql-multitenancy) markers:
* query_results + sync policy_membership stamp from the request team pin
(fleet.OpenframeTeamID(ctx) — the agent plane is always pinned in shared
mode);
* async policy_membership + activity_host_past stamp from the row's own host
via (SELECT team_id FROM hosts WHERE id = ?) — those run from unpinned
cron contexts;
* activity_past stamps from the request pin — the only tenant signal for
host-less (user/team-level) activities.
Backward compatible: unpinned writes (flag off, or background crons writing
host-less activities) keep the statements byte-identical and leave team_id
NULL. No SELECT * struct-scan readers exist on these tables, so the added
column cannot break reads. Verified with MySQL-backed tests (migration
idempotency + stamping across pinned/unpinned/async paths) and flag-off
regressions.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- policies.go: wrap the team_id-carrying policy_membership INSERT in the >>> / <<< OPENFRAME(mysql-multitenancy) sentinel markers so the grep-based fork-audit convention finds it on upstream merges (was a bare comment). - new_activity_test.go: scope the activity_past lookup to the test's own user_id + ORDER BY id DESC LIMIT 1 so it stays deterministic regardless of subtest isolation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
oleksandrd-flamingo
force-pushed
the
feature/cdc-team-id-stamping
branch
from
July 27, 2026 12:29
fc98567 to
8a0960e
Compare
oleksandrd-flamingo
enabled auto-merge (squash)
July 27, 2026 12:30
Arsenii-Malov
approved these changes
Jul 27, 2026
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Under Fleet shared-DB multi-tenancy (one MySQL serving every tenant, gated by
FLEET_OPENFRAME_MULTI_TENANCY_ENABLED), a CDC record must carry its own tenant discriminator: the Debezium connector's SMTs are stateless and cannot joinhost_id → hosts.team_id, andactivity_pastrows have no host reference at all. This PR stamps ateam_idcolumn onto the four CDC-captured tables so the shared-plane stream consumer can resolve the tenant per event (the platform-side connector + shared-stream consumption land in theopenframe-saas-sharedrepo).This is the CDC foundation of the Debezium-on-shared-MySQL work; it is deliberately option-agnostic (both the shared-consumption and fan-out designs build on it).
Changes
Migration —
20260722000001_AddTeamIdToCdcTables(fork's separateopenframegoose pipeline, not upstream lineage):team_id INT UNSIGNED NULLonactivity_past,activity_host_past,query_results,policy_membership.columnExistsguards.Write-path stamping — all wrapped in
OPENFRAME(mysql-multitenancy)sentinel markers:query_resultsfleet.OpenframeTeamID(ctx))OverwriteQueryResultRowspolicy_membership(sync)RecordPolicyQueryExecutionspolicy_membership(async collector)(SELECT team_id FROM hosts WHERE id = ?)(cron context, never pinned)AsyncBatchInsertPolicyMembershipactivity_pastserver/activity/internal/mysql/new_activity.goactivity_host_pastBackward compatibility
team_id = NULL; separate goose pipeline (migration_status_openframe) so upstream sync lineage is untouched.SELECT *into a fixed struct on these tables.team_idNULL. Releasing to the current per-tenant MySQL behaves exactly like fork-main; flipping the flag later starts populating the column.Testing
MySQL-backed (
MYSQL_TEST=1):TestMigrateOpenframeCdcTeamIdStamping— migration idempotency (all four columns) + stamping across pinned (query_results), unpinned-NULL, and async-from-host (policy_membership) paths.TestNewActivity/OpenframeTeamStamping—activity_paststamped from the request pin,activity_host_paststamped from the host's own team via subselect.TestQueryResults,TestPolicies/RecordPolicyQueryExecutions,TestPolicies/ResetAttemptsOnFailingToPassing{Sync,Async}, fullTestNewActivity.go build ./server/... ./cmd/fleetclean;gofmtclean.Notes
openframe/docs/mysql-multitenancy-feature.md(new "CDC team stamping" section + migration row) andopenframe/docs/fork-file-manifest.md.ctx-pin || IsOpenframeMultitenancy()so the MySQL-backed tests can pin via context withoutt.Setenv(the parallel harness forbids it); production behavior is identical either way — the SQL readshosts.team_id, ignoring the pin value.🤖 Generated with Claude Code
Summary by CodeRabbit
team_idacross relevant tables.