Skip to content

fix: project rig dolt env into exec-backed controller stores#1767

Merged
sjarmak merged 3 commits intogastownhall:mainfrom
azanar:upstream-main-fix-rig-store-dolt-env
May 8, 2026
Merged

fix: project rig dolt env into exec-backed controller stores#1767
sjarmak merged 3 commits intogastownhall:mainfrom
azanar:upstream-main-fix-rig-store-dolt-env

Conversation

@azanar
Copy link
Copy Markdown
Contributor

@azanar azanar commented May 6, 2026

Summary

This fixes rig-scoped exec-backed controller stores so they receive the same projected Dolt environment that one-shot scoped store opens already receive.

What changed

  • update controllerState.openRigStore() to inject scoped Dolt host/port env for exec providers that use the bd store contract
  • add a focused regression test for exec:gc-beads-bd rig stores opened through the controller path

Why

openStoreAtForCity() already projects scoped Dolt env for exec:gc-beads-bd, but the controller's long-lived rig store path did not. That let rig-scoped cached/controller store operations diverge from direct gc bd --rig ... behavior and still resolve to 127.0.0.1:0 in later reconcile/order-dispatch paths.

This makes the controller path use the same scoped target projection as the direct store-open path.

Tests

  • go test ./cmd/gc -run 'TestControllerState(OpenRigStoreExecProjectsRigTarget|OpenRigStoreExecBdProjectsRigDoltEnv)|TestOpenStoreAtForCityExecUsesUniversalStoreTargetEnv' -count=1

View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

@azanar azanar requested a review from julianknutsen as a code owner May 6, 2026 22:40
@github-actions github-actions Bot added the status/needs-triage Inbox — we haven't looked at it yet label May 6, 2026
@randy-release-manager randy-release-manager Bot added kind/bug Broken behavior priority/p1 High — core workflow broken and removed status/needs-triage Inbox — we haven't looked at it yet labels May 6, 2026
Copy link
Copy Markdown
Contributor

@sjarmak sjarmak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @azanar — direct parity port of the openStoreAtForCity rig branch into controllerState.openRigStore(). Eliminates the 127.0.0.1:0 divergence on cached/controller rig-store ops without expanding blast radius, and reuses the in-hand cfg instead of loadCityConfig-ing again (small efficiency win over the direct path). The positive case is covered by TestControllerStateOpenRigStoreExecProjectsRigTarget; the negative case (non-bd exec providers don't get the projection) is implicitly covered by the same test's leak check. Good to merge.

Sibling-batch context: complementary to #1766 (which hardens the upstream resolvedRuntimeCityDoltTarget that bdRuntimeEnvForRig resolves through). No conflict; merge order doesn't matter.

Future cleanup (don't block on it): the two rig branches now share ~12 lines of env-projection logic — worth an applyExecRigDoltEnv(...) helper once a third caller appears.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sjarmak sjarmak merged commit 487521f into gastownhall:main May 8, 2026
63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Broken behavior priority/p1 High — core workflow broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants