refactor: unify connection storage on agent_connections, drop chat_connections#506
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
buremba
added a commit
that referenced
this pull request
May 4, 2026
…efactor Squash-rebase of PR #506 onto current main (post #499 package rename owletto-backend → server). Combines the original 7-commit history into one because the rename touched every file in the diff. What changed: - ChatInstanceManager reads/writes agent_connections directly via AgentConnectionStore. Drops the parallel chat_connections table + ChatConnectionStore shim. One source of truth. - Connection secrets live as secret:// refs inside the row's config JSON, resolved at runtime through SecretStoreRegistry. Same model as every other secret category — pluggable to AWS Secrets Manager / Vault / k8s without per-platform code paths. - normalizeConfigForStorage moves plaintext secrets into the registry via secretStore.put('connections/<id>/<field>') before save. Idempotent (already-ref values pass through). - resolveConfigForRuntime materializes refs back to values inside startInstance, throws on unresolvable refs. - removeConnection cascades deleteSecretsByPrefix in safe order (history → secrets → row); addConnection rollback uses the same order. - updateConnection's needsRestart resolves previous refs before comparing against caller plaintext, so idempotent lobu-apply re-runs don't trip spurious restarts. - Migration: copies any chat_connections rows into agent_connections (filtering NULL template_agent_id orphans) before DROP. Embedded PGlite path mirrors the same copy+drop wrapped in a pg_tables existence check so fresh installs skip cleanly. - agent-routes.ts: no-manager fallback writes return 503 instead of persisting plaintext directly into agent_connections.config. - postgres-stores.ts: stops encrypting writes; persists config JSON as-is. decryptLegacyEncryptedConfig fallback keeps any pre-existing enc:v1: rows decryptable for graceful transition. - platform.ts: getConnectionStore() added to CoreServices interface. - Bundled small fixes: Telegram non-fatal setWebhook, http-proxy early error handlers, embedded-deployment env hygiene, core-services LOBU_PROVIDER_REGISTRY_PATH respect, agent_channel_bindings startup no longer deletes connections on secret resolution failure. - Drive-by typecheck fixes for pre-existing root-tsc errors in agent-worker that blocked the pre-commit hook (unrelated to this PR's scope, but required to land cleanly): - just-bash-bootstrap.ts: cast envRecord to NodeJS.ProcessEnv for execFile's env option (Vite ImportMetaEnv augments ProcessEnv with BASE_URL/MODE/DEV/PROD/SSR fields stripEnv's return type doesn't include). - openclaw/tools.ts: same cast on the spawnHook env return. Verified: - Per-package bun run typecheck exit 0. - Root bunx tsc --noEmit exit 0. - bun test src/gateway/__tests__/ src/lobu — 559 pass / 0 fail.
buremba
force-pushed
the
refactor/unify-connection-storage
branch
from
bc4f609 to
f422d47
Compare
github-actions
Bot
added
the
triage:needs-human
Contributor
|
Triage decision: Reasons:
Next: Address failing checks, then merge manually after final review. Large PRs require human oversight regardless of bot approval status. |
…efactor Squash-rebase of PR #506 onto current main (post #499 package rename owletto-backend → server). Combines the original 7-commit history into one because the rename touched every file in the diff. What changed: - ChatInstanceManager reads/writes agent_connections directly via AgentConnectionStore. Drops the parallel chat_connections table + ChatConnectionStore shim. One source of truth. - Connection secrets live as secret:// refs inside the row's config JSON, resolved at runtime through SecretStoreRegistry. Same model as every other secret category — pluggable to AWS Secrets Manager / Vault / k8s without per-platform code paths. - normalizeConfigForStorage moves plaintext secrets into the registry via secretStore.put('connections/<id>/<field>') before save. Idempotent (already-ref values pass through). - resolveConfigForRuntime materializes refs back to values inside startInstance, throws on unresolvable refs. - removeConnection cascades deleteSecretsByPrefix in safe order (history → secrets → row); addConnection rollback uses the same order. - updateConnection's needsRestart resolves previous refs before comparing against caller plaintext, so idempotent lobu-apply re-runs don't trip spurious restarts. - Migration: copies any chat_connections rows into agent_connections (filtering NULL template_agent_id orphans) before DROP. Embedded PGlite path mirrors the same copy+drop wrapped in a pg_tables existence check so fresh installs skip cleanly. - agent-routes.ts: no-manager fallback writes return 503 instead of persisting plaintext directly into agent_connections.config. - postgres-stores.ts: stops encrypting writes; persists config JSON as-is. decryptLegacyEncryptedConfig fallback keeps any pre-existing enc:v1: rows decryptable for graceful transition. - platform.ts: getConnectionStore() added to CoreServices interface. - Bundled small fixes: Telegram non-fatal setWebhook, http-proxy early error handlers, embedded-deployment env hygiene, core-services LOBU_PROVIDER_REGISTRY_PATH respect, agent_channel_bindings startup no longer deletes connections on secret resolution failure. - Drive-by typecheck fixes for pre-existing root-tsc errors in agent-worker that blocked the pre-commit hook (unrelated to this PR's scope, but required to land cleanly): - just-bash-bootstrap.ts: cast envRecord to NodeJS.ProcessEnv for execFile's env option (Vite ImportMetaEnv augments ProcessEnv with BASE_URL/MODE/DEV/PROD/SSR fields stripEnv's return type doesn't include). - openclaw/tools.ts: same cast on the spawnHook env return. Verified: - Per-package bun run typecheck exit 0. - Root bunx tsc --noEmit exit 0. - bun test src/gateway/__tests__/ src/lobu — 559 pass / 0 fail.
buremba
force-pushed
the
refactor/unify-connection-storage
branch
from
f422d47 to
44914b4
Compare
buremba
added a commit
that referenced
this pull request
May 4, 2026
Biome wants the stripEnv call on a single line. Fixes the format-lint failure on main introduced by #506.
buremba
added a commit
that referenced
this pull request
May 4, 2026
#506's chat-instance-manager.ts:initialize loads connections at gateway boot and calls startInstance() → resolveConfigForRuntime() → secretStore.get(). Since #516 made PostgresSecretStore resolve secrets per-org via AsyncLocalStorage, boot-time reads have no org context, fall back to the GLOBAL bucket, and find nothing — every connection written under an org's context is then marked status='error' on first restart. Resolve the agent's organization_id via getAgentOrganizationId() and wrap the startInstance call in orgContext.run({organizationId}). Pre-org rows (no template agent or no org) keep the GLOBAL bucket fallback. Verified: bun test packages/server/src/gateway → 502 pass / 0 fail.
buremba
added a commit
that referenced
this pull request
May 4, 2026
… webhooks (#522) * fix(chat-instance): wrap boot-time startInstance in orgContext #506's chat-instance-manager.ts:initialize loads connections at gateway boot and calls startInstance() → resolveConfigForRuntime() → secretStore.get(). Since #516 made PostgresSecretStore resolve secrets per-org via AsyncLocalStorage, boot-time reads have no org context, fall back to the GLOBAL bucket, and find nothing — every connection written under an org's context is then marked status='error' on first restart. Resolve the agent's organization_id via getAgentOrganizationId() and wrap the startInstance call in orgContext.run({organizationId}). Pre-org rows (no template agent or no org) keep the GLOBAL bucket fallback. Verified: bun test packages/server/src/gateway → 502 pass / 0 fail. * chore: regen lockfile post-rebase onto #508
5 tasks
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
Collapses two parallel connection-storage tables into one.
ChatInstanceManagernow reads/writesagent_connectionsdirectly viaAgentConnectionStoreinstead of the separatechat_connectionstable +ChatConnectionStoreshim. Secret fields stay behindsecret://ref indirection throughSecretStoreRegistryso chat-platform tokens can be backed by Postgres / AWS Secrets Manager / Vault / k8s — same as every other secret category.Commits
b1eef69d— unify connection storageChatInstanceManagerswitches fromChatConnectionStoretoAgentConnectionStore.persistConnectionSnapshotdual-write fromagent-routes.ts.chat_connectionstable.listConnections()usestryGetOrgId()so the manager can load connections at startup without org context.LOBU_PROVIDER_REGISTRY_PATH;agent_channel_bindingsstartup no longer deletes connections on secret resolution failure.enc:v1:blobs — undone by the41eb30a2commit below.3dc03517— delete deadChatConnectionStorefile (-128 lines).1f641b1a— fix stalepersistConnectionSnapshotcomment in test.e1f55bf7— Telegram:setWebhookfailure no longer fatal at adapter startup; the adapter can still receive messages via an existing webhook configuration.41eb30a2— restore secret-ref indirection for connection secretsChatInstanceManager.normalizeConfigForStoragemoves plaintext secrets into the registry viasecretStore.put("connections/<id>/<field>")and replaces values withsecret://...refs before save. Idempotent — already-ref values pass through.ChatInstanceManager.resolveConfigForRuntimematerializes refs back to values insidestartInstancebefore handing config to the Chat SDK adapter. Throws on unresolvable refs so a wiped secret surfaces as an errored connection rather than a half-baked boot.removeConnectioncascadesdeleteSecretsByPrefix("connections/<id>/")so torn-down connections don't leak secret rows. Add-connection rollback path also cleans up.postgres-stores.tssaveConnectionno longer encrypts — persists config JSON as-is. Read-sidedecryptLegacyEncryptedConfigkeeps any pre-existingenc:v1:rows decryptable during transition; new writes never produce them.getConnectionStore()added toCoreServices,stored.settingscast atstoredToPlatform. Per-packagebun run typecheckis now clean (the root tsconfig excludes owletto-backend, so PR-time checks weren't catching these).PostgresSecretStore-backed ref so it actually exercises the new resolution path.Net effect
~370 LOC net removed in the unification, restored ~165 LOC of ref-indirection (still net negative). One source of truth for connection rows. Connection secrets are now uniformly behind
SecretStoreRegistry— same model as every other secret category, swappable backend.Independent of #505 — file-overlap is
core-services.tsonly (one-line addition vs. #505's removals; clean rebase).Test plan
bun run typecheckpasses (was failing on this branch before — pre-existing issues fixed in41eb30a2).bun run typecheckpasses.bun test src/gateway/__tests__/— 515 pass, 0 fail.bun test src/lobu— 44 pass, 0 fail.chat-instance-manager-slackreload test exercises real ref resolution (PostgresSecretStore + SecretStoreRegistry).chat_connectionsmigration applies cleanly on existing deployments with chat-platform connections (decryptLegacyEncryptedConfig handles in-flightenc:v1:rows from anyone who built mid-PR).secretsDeletedcount > 0 inremoveConnectionlog when the connection has tokens.