Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 846cfa3102
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const runSqliteProviderServiceCatalogRepair = ( | ||
| client: SqliteDataMigrationClient, | ||
| ): Effect.Effect<number, DataMigrationError> => |
There was a problem hiding this comment.
This repair is implemented only for SqliteDataMigrationClient and registered by the local, self-host, and D1 hosts. A repo-wide search of apps/cloud/drizzle and apps/cloud/scripts/code-migrations finds no PostgreSQL counterpart, even though the cloud provider-service split was applied by scripts/migration/service-split-postgres-cli.ts. Consequently, affected cloud rows retain their non-null tools_synced_at, never enter resolveTools, and remain missing from the catalog after this deployment.
Useful? React with 👍 / 👎.
| current, | ||
| storage: input.storage, | ||
| httpClientLayer: options?.httpClientLayer ?? input.httpClientLayer, | ||
| }).pipe(Effect.catch(() => Effect.succeed(undefined))); |
There was a problem hiding this comment.
When a non-admin member is the first reader after this migration, authorizeWrite() fails here but the catch converts that denial into a successful incomplete result. Core's incomplete-catalog path then stamps tools_synced_at, so the connection is no longer stale; a later integration administrator's ordinary tools read will therefore not call this resolver and cannot perform the promised repair. Let the authorization failure reach the stale-sync best-effort boundary, or otherwise keep the connection stale for a subsequent administrator.
Useful? React with 👍 / 👎.
| ...(sourceSpecHash ? { sourceSpecHash } : {}), | ||
| specUrl: resolved.specUrl ?? specUrl, | ||
| }; | ||
| yield* input.ctx.transaction( |
There was a problem hiding this comment.
If an affected integration has two or more stale connections, syncStaleConnectionTools intentionally resolves them concurrently, so each can observe an empty operation catalog and open this transaction simultaneously. On the self-host/local single-connection libSQL path, overlapping transactions fail with cannot start a transaction within a transaction; the failure is then swallowed by the new catch and that connection can be sync-stamped without repaired tools, or can persist the pre-repair stored catalog after a peer populates operations. Gate this integration-global repair with a per-integration single-flight or move its writes behind the existing serialized persistence phase.
Useful? React with 👍 / 👎.
Withdrawn v1 provider catalog proposal
This proposal targets the v1 provider-service tables. The current v2 app/deployment catalog uses a different storage model, so this migration is not applicable to current v2 and should not be merged.
Reconciled against
UsefulSoftwareCo/executorbranchv2at0e9e2c6b376b9f061b36ffdf33b657be08a557b5(October 13). Four named self-host E2E scenarios passed on that revision: scoped access/live edits/revocation, exact account targets, dashboard connection creation/revocation, and updated tools after redeploy without relinking accounts. Tests used isolated product servers, scratch databases and synthetic fixtures. No release or deployment occurred.The proposal's source commit remains preserved. This closed PR is historical v1 work, not a current v2 delivery.