Conversation
- Add MongoDB driver dependency and configuration for persisting database targets. - Introduce BEDROCK_MONGO_URI, BEDROCK_MONGO_DATABASE, BEDROCK_MONGO_TARGETS_COLLECTION environment variables. - Add BEDROCK_API_BEARER_TOKEN for HTTP API authentication. - Add BEDROCK_DB_TEST_ALLOWED_HOSTS to restrict hosts for connection tests. - Update docker-compose with MongoDB service and required environment variables. - Update README with new CLI option and environment variables.
- Add react-helmet-async dependency for managing page titles - Implement AppBreadcrumbs component and active route title extraction - Update AppShell to display breadcrumbs and set document title based on current route
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR adds Mongo-backed persistence for targets and SQL files, static API bearer-token auth, Postgres drift analysis with control summaries, a SQL-files management UI, breadcrumb/title routing metadata, and supporting docs, Docker, and build updates. ChangesBackend: persistence, auth, drift, and SQL-file routes
Frontend: selected target, SQL files page, drift, schema, breadcrumbs
Docs, Docker, nginx, and build config
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SqlFileRoutes
participant SqlFileStore
participant SchemaRoutes
participant PostgresDriftAnalyzer
Client->>SqlFileRoutes: POST /sql-files/upload-zip
SqlFileRoutes->>SqlFileRoutes: extract and normalize .sql entries
SqlFileRoutes->>SqlFileStore: replaceAll(files)
SqlFileStore-->>SqlFileRoutes: ack
Client->>SchemaRoutes: GET /drift
SchemaRoutes->>SqlFileStore: toSqlFiles()
SqlFileStore-->>SchemaRoutes: List[SqlFile]
SchemaRoutes->>PostgresDriftAnalyzer: mergeCatalog, driftItems
PostgresDriftAnalyzer-->>SchemaRoutes: DriftResponse data
sequenceDiagram
participant Client
participant JwtMiddleware
participant JwtTokens
Client->>JwtMiddleware: request with Authorization header
JwtMiddleware->>JwtMiddleware: constantTimeEquals(token, apiBearerToken)
alt static token matches
JwtMiddleware-->>Client: authorized
else
JwtMiddleware->>JwtTokens: verify(jwtSecret, token)
JwtTokens-->>JwtMiddleware: Claims or error
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (10)
AGENTS.md (2)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded personal machine path in scope declaration.
This file governs \/Users/rcs/git/wiretrap/schema-migrator`.bakes in a specific developer's local clone path (usernamercs, underwiretrap`). If this file is used by AI agents/tools to scope their work, this absolute path won't match any other contributor's or CI's checkout location, making the scope declaration ineffective outside this one machine.📝 Suggested fix using a repo-relative reference
-This file governs `/Users/rcs/git/wiretrap/schema-migrator`. +This file governs the `schema-migrator` repository (this directory and its subdirectories).🤖 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 `@AGENTS.md` at line 4, The scope declaration in AGENTS.md uses a hardcoded absolute local path, which makes it specific to one machine. Replace that path with a repo-relative reference or a generic workspace-relative description in the governing scope text so tools and agents can resolve it correctly in any checkout; update the scope statement itself rather than adding another machine-specific path.
13-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMarkdown list-indent warnings (MD005).
markdownlint flags these list items for inconsistent indentation (leading space before
-) versus sibling items at the same level.Also applies to: 31-36
🤖 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 `@AGENTS.md` around lines 13 - 16, Fix the markdownlint MD005 warnings by making the list indentation consistent in AGENTS.md. The affected bullet items in the schema-migrator-ui/docker-compose description and the similar section noted elsewhere should use the same leading indentation as sibling list entries, with no extra leading space before the hyphen. Update the surrounding markdown list formatting so the nested bullets remain structurally correct and consistent across the document.Source: Linters/SAST tools
src/main/scala/com/sslproxy/schema/store/SqlFileStore.scala (2)
114-118: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRedundant secondary index on
path.
_idis already set tofile.path(Line 122), giving an implicit unique index on that value. The separate ascending index on the"path"field indexes the same data again under a different key and adds write overhead without new query capability.🤖 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 `@src/main/scala/com/sslproxy/schema/store/SqlFileStore.scala` around lines 114 - 118, Remove the redundant secondary index creation in the SqlFileStore.initialize method: keep the compound index on folder and filename, but drop the separate Indexes.ascending("path") call because file.path is already stored as _id in the same store and is implicitly indexed. Update the initialize logic in SqlFileStore so it only creates the index that is still needed.
97-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
toSqlFilesimplementation across both store variants.The Mongo and in-memory implementations of
toSqlFilesare byte-for-byte identical. Extracting a shared helper (e.g., a default method on theSqlFileStoretrait built on top oflist) would avoid the two copies drifting apart.♻️ Suggested consolidation
trait SqlFileStore: ... - def toSqlFiles: IO[List[SqlFile]] + def toSqlFiles: IO[List[SqlFile]] = list.flatMap(toSqlFilesFrom) + + protected def toSqlFilesFrom(storedList: List[StoredSqlFile]): IO[List[SqlFile]] = + storedList.traverse { stored => + IO.delay { + val bytes = Base64.getDecoder.decode(stored.contentBase64) + SqlFile( + folder = stored.folder, + path = Path.of(stored.path), + name = stored.filename, + relativePath = stored.path, + content = Some(new String(bytes, StandardCharsets.UTF_8)) + ) + } + }Then remove the two duplicated
override def toSqlFilesbodies.Also applies to: 161-176
🤖 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 `@src/main/scala/com/sslproxy/schema/store/SqlFileStore.scala` around lines 97 - 112, The `toSqlFiles` logic is duplicated in both `SqlFileStore` implementations, so extract the shared conversion into a common helper on the `SqlFileStore` trait using `list` as the source and remove the two concrete `override def toSqlFiles` bodies. Keep the shared decoding and `SqlFile` construction in one place so the Mongo and in-memory stores both reuse the same implementation and cannot drift apart.src/main/scala/com/sslproxy/schema/cli/CliOpts.scala (1)
105-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePartial Mongo config collapses to a generic error.
(mongoUri, mongoDatabase, mongoTargetsCollection).mapN(MongoConfig.apply)silently drops toNoneif only some of the three Mongo options are set, so a user who sets--mongo-uribut forgets--mongo-databasegets the same "must be set" message as someone who set nothing, rather than being told which specific value is missing.Consider validating the three independently and surfacing the missing field name(s) for faster diagnosis.
🤖 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 `@src/main/scala/com/sslproxy/schema/cli/CliOpts.scala` around lines 105 - 172, The Mongo config assembly in `serverOpts` currently uses `(mongoUri, mongoDatabase, mongoTargetsCollection).mapN(MongoConfig.apply)`, which turns any partially provided set of options into a generic `None`. Change the validation around `mongoUriOpt`, `mongoDatabaseOpt`, and `mongoTargetsCollectionOpt` so each field is checked independently and missing values are reported by name when any Mongo setting is incomplete. Keep the final `ServerConfig` wiring intact, but replace the silent `mapN` collapse with explicit validation logic that preserves a clear error for the specific absent option(s)..bsp/sbt.json (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMachine-specific generated file committed to the repo.
This BSP config embeds absolute local paths (JDK install dir, IntelliJ version, and a local username) specific to one contributor's machine. It will be invalid/misleading for other contributors and shouldn't be tracked.
🧹 Suggested fix
+.bsp/Add to
.gitignoreand remove.bsp/sbt.jsonfrom the repo (git rm --cached .bsp/sbt.json).🤖 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 @.bsp/sbt.json at line 1, The committed BSP config is machine-specific and should not be tracked because it contains absolute local paths and user-specific IntelliJ/JDK values. Remove the generated `.bsp/sbt.json` from version control, add `.bsp/` or this file to `.gitignore`, and keep the repo clean by untracking the file while leaving local generation to each contributor’s environment.docker-compose.yml (1)
3-16: 🔒 Security & Privacy | 🔵 TrivialConsider enabling Mongo authentication for defense-in-depth.
The
mongoservice runs without root credentials. It's not published to the host, but addingMONGO_INITDB_ROOT_USERNAME/PASSWORD(and matching credentials inBEDROCK_MONGO_URI) would harden the setup against lateral movement within theinternalnetwork.🤖 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 `@docker-compose.yml` around lines 3 - 16, The mongo service is running without authentication, so harden it by enabling Mongo root credentials in the mongo service configuration and updating any client connection string to use them. Add the appropriate initialization environment variables for the mongo container, then update BEDROCK_MONGO_URI so services connect with matching credentials; keep the existing mongo service and healthcheck setup aligned with the authenticated connection.schema-migrator-ui/src/hooks/useSelectedTarget.tsx (1)
60-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding direct unit tests for this hook.
useSelectedTargetId/SelectedTargetProvideris now shared infrastructure across Drift, Patches, Runs, Schema and TargetSelector. Coverage today is indirect, only exercised through consumer component tests. A focused test (URL-vs-storage precedence, cross-tabstorageevent sync, invalid/empty normalization) would make regressions in this shared state easier to catch.🤖 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 `@schema-migrator-ui/src/hooks/useSelectedTarget.tsx` around lines 60 - 72, Add direct unit tests for useSelectedTargetId and the SelectedTargetProvider shared state behavior. Cover URL parameter precedence over stored selection, synchronization via the browser storage event across tabs, and normalization of empty/invalid values to null. Place the tests near the useSelectedTarget hook so regressions in this shared infrastructure are caught without relying only on consumer component tests.schema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.tsx (1)
414-530: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a well-tested zip library instead of a hand-rolled writer.
The manual ZIP writer (local/central headers, CRC32, EOCD) is correct as reviewed, but hand-rolled binary-format code is easy to regress and hard to extend (e.g., no compression, no Zip64 for very large manifests). Libraries like
fflateorclient-zipprovide this with a smaller, well-tested surface area.🤖 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 `@schema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.tsx` around lines 414 - 530, Replace the hand-rolled ZIP assembly in ZipWriter with a well-tested client-side ZIP library such as fflate or client-zip. Keep the ZipWriter API shape if needed, but move the binary header/CRC/EOCD logic out of toBytes() and delegate archive creation to the library so future extensions like compression and Zip64 are handled safely.src/main/scala/com/sslproxy/schema/server/DbPing.scala (1)
12-12: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winWiring to
JdbcConnectionSettingsand redaction logic look correct.Consider adding unit tests for
connectionError/redact(e.g. asserting apostgres://user:secret@hostembedded in an exception message is redacted, and thatpassword=/pwd=query values are stripped) since this is the safeguard against leaking credentials in connection-test error responses returned to clients.Also applies to: 28-33, 45-47, 72-90
🤖 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 `@src/main/scala/com/sslproxy/schema/server/DbPing.scala` at line 12, Add unit tests around DbPing’s connectionError and redact helpers to cover the credential-scrubbing behavior. Verify that exception messages containing embedded database URLs like postgres://user:secret@host are redacted before being returned, and that query parameters such as password= and pwd= are stripped or masked. Use the existing DbPing/connectionError/redact flow so the tests exercise the same safeguard used for client-facing connection-test errors.
🤖 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 `@docker-compose.yml`:
- Around line 26-36: The default value for BEDROCK_DB_TEST_ALLOWED_HOSTS in
docker-compose.yml includes a hardcoded private LAN IP, which should not be
committed as a shared default. Update the environment entry to use only generic
local test hosts, and if that host is still needed, make it explicitly
configurable via an override rather than baked into the default. Keep the change
focused on the BEDROCK_DB_TEST_ALLOWED_HOSTS setting in the compose environment
block.
In `@nginx/nginx.conf.template`:
- Around line 1-4: The authorization fallback is currently defined in the
top-level map for $schema_migrator_authorization, which causes
${BEDROCK_API_BEARER_TOKEN} to be added to all proxied requests including /api/.
Update the nginx config so /api/ remains a direct passthrough of
$http_authorization, and move the bearer-token fallback logic into the
^/api/runs/[^/]+/stream$ location only. Use the existing map and location blocks
in nginx.conf.template to scope the fallback narrowly without changing other
request paths.
In `@schema-migrator-ui/package.json`:
- Around line 41-43: Remove the unnecessary workspace declaration from the
standalone manifest by updating the package.json for schema-migrator-ui so it no
longer defines workspaces with ".". This change should be made directly in the
package manifest and verified so npm treats the package as independent; if root
inclusion is needed for workspace commands, rely on npm’s
--include-workspace-root behavior instead of listing the root package in
workspaces.
In `@schema-migrator-ui/src/pages/Drift/DriftPage.tsx`:
- Line 1: The drift-type chip counts are computed from the unfiltered items set,
so they do not match the active text search; update the count calculation in
DriftPage so it uses the same text-filtered items as the table. Adjust the
memoized logic around driftCounts, items, and textFilter so the counts are
derived from the already text-filtered dataset rather than data?.items alone.
In `@schema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.tsx`:
- Around line 244-247: The directory picker trigger in SqlFilesPage is using a
focusable label with role="button", but it still won’t open the chooser via
Enter/Space for keyboard users. Update the control to use a native button-style
trigger, or add explicit keyboard handlers for the existing label so activation
works from the keyboard as well as the mouse. Keep the existing behavior around
the sql-dir-picker input and uploading state, but make the trigger in the
SqlFilesPage render path keyboard-activatable.
In `@src/main/scala/com/sslproxy/schema/db/postgres/PostgresProvider.scala`:
- Around line 179-188: `parsePostgresJdbcUrl` is bypassing the host check for
JDBC URLs without credentials, so malformed inputs can slip through `normalize`
and `TargetRoutes.validateTargetPayload`. Update the JDBC shortcut path in
`PostgresProvider.normalize`/`parsePostgresJdbcUrl` to validate that a host is
present before returning `JdbcConnectionConfig`, or route these inputs through
`parsePostgresUri` so the existing host-required logic is applied consistently.
In `@src/main/scala/com/sslproxy/schema/server/compress/Bzip2.scala`:
- Around line 72-75: The error handling in Bzip2’s stream recovery is
incorrectly surfacing incidental writer failures instead of preserving malformed
input as BadInput. In Bzip2.scala, update the failure branch in the
writerFailure.tryGet handling so that invalid compressed bodies always raise
BadInput with the original error, and only let true SizeLimitExceeded cases
propagate separately; do not rethrow writerError from this path because it can
bypass Bzip2Middleware’s BadInput mapping.
In `@src/main/scala/com/sslproxy/schema/server/compress/Bzip2Middleware.scala`:
- Around line 80-91: The responseWithCompressionDecision method buffers the
entire body when Content-Length is missing, which can blow up memory and break
streaming; update the Bzip2Middleware flow to avoid compiling response.body into
a Vector. Instead, either decide compression without full buffering or only peek
up to thresholdBytes + 1 from the stream, and keep the unknown-length path
streaming-friendly while preserving the existing compression decision logic.
In `@src/main/scala/com/sslproxy/schema/server/PostgresDriftAnalyzer.scala`:
- Around line 243-281: PreparedState.apply is using sourceFile as a fallback in
a way that can pick the wrong ControlObject when multiple controls share the
same file. Update PreparedState.apply and its
controlBySource/expectedControlByKey logic so a missing exact key only falls
back to an unambiguous match, or otherwise skips the fallback rather than
reusing a sibling control. Keep the behavior for controlForKey, mergeCatalog,
and driftItems consistent with the intended exact-key resolution, and add
coverage for grouped sourceFile cases to verify the fallback does not carry over
the wrong expectedDdl or applyStatus.
In `@src/main/scala/com/sslproxy/schema/server/SchemaRoutes.scala`:
- Around line 318-321: The catalog key built in SchemaRoutes is currently using
only p.proname, which can merge overloaded functions/procedures in the same
schema. Update the query/object_name construction in the relevant SchemaRoutes
method so it includes the function/procedure signature or identity arguments
alongside the name, preserving uniqueness for pg_get_functiondef-based
comparisons. Ensure the change keeps each overloaded routine distinct in the
catalog key used for drift detection.
In `@src/main/scala/com/sslproxy/schema/server/SqlFileRoutes.scala`:
- Around line 147-161: The file upload flow in fileUploads currently accepts any
multipart filename, which lets non-.sql content reach normalization and manifest
parsing. Add a validation step in fileUploads (before
safeFilename/SqlPathNormalizer.normalizeUploadPath) to reject uploads whose
part.filename does not end with .sql, and return an appropriate upload error.
Use the existing fileUploads, safeFilename, and SqlPathNormalizer symbols to
keep the check close to the current upload handling.
- Around line 68-83: In SqlFileRoutes, the upload flow built around
request.as[Multipart[IO]] and fileUploads currently proceeds to
sqlFileStore.replaceAll(stored) even when stored is empty, which can overwrite
the manifest with nothing. Add an explicit empty-upload guard before replaceAll
in the same route logic, similar to the zip route’s empty-archive check, so that
Nil from fileUploads is rejected and only non-empty uploads reach
StoredSqlFile.fromBytes and sqlFileStore.replaceAll.
In `@src/main/scala/com/sslproxy/schema/store/MongoTargetStore.scala`:
- Around line 72-93: The Mongo target persistence path is storing and retrieving
the database password in plaintext via MongoTargetStore.documentFor and
MongoTargetStore.storedFromDocument. Change this flow so StoredTarget.password
is saved as encrypted ciphertext or a secret reference in MongoDB, and only
decrypted later when building the JDBC connection, keeping the document schema
and any read path in sync with the new secure representation.
In `@src/main/scala/com/sslproxy/schema/store/SqlFileStore.scala`:
- Around line 82-89: `SqlFileStore.replaceAll` is non-atomic because it calls
`deleteMany` before `insertMany`, so a failure in the second step can erase the
stored manifest; update this method to use an atomic or staged swap approach
instead. Prefer a MongoDB transaction in `replaceAll` if supported, or write the
new `StoredSqlFile` documents first and only delete stale records after the new
writes succeed, using the existing `collection` and `toDocument` flow so the
manifest is never left empty on partial failure.
---
Nitpick comments:
In @.bsp/sbt.json:
- Line 1: The committed BSP config is machine-specific and should not be tracked
because it contains absolute local paths and user-specific IntelliJ/JDK values.
Remove the generated `.bsp/sbt.json` from version control, add `.bsp/` or this
file to `.gitignore`, and keep the repo clean by untracking the file while
leaving local generation to each contributor’s environment.
In `@AGENTS.md`:
- Line 4: The scope declaration in AGENTS.md uses a hardcoded absolute local
path, which makes it specific to one machine. Replace that path with a
repo-relative reference or a generic workspace-relative description in the
governing scope text so tools and agents can resolve it correctly in any
checkout; update the scope statement itself rather than adding another
machine-specific path.
- Around line 13-16: Fix the markdownlint MD005 warnings by making the list
indentation consistent in AGENTS.md. The affected bullet items in the
schema-migrator-ui/docker-compose description and the similar section noted
elsewhere should use the same leading indentation as sibling list entries, with
no extra leading space before the hyphen. Update the surrounding markdown list
formatting so the nested bullets remain structurally correct and consistent
across the document.
In `@docker-compose.yml`:
- Around line 3-16: The mongo service is running without authentication, so
harden it by enabling Mongo root credentials in the mongo service configuration
and updating any client connection string to use them. Add the appropriate
initialization environment variables for the mongo container, then update
BEDROCK_MONGO_URI so services connect with matching credentials; keep the
existing mongo service and healthcheck setup aligned with the authenticated
connection.
In `@schema-migrator-ui/src/hooks/useSelectedTarget.tsx`:
- Around line 60-72: Add direct unit tests for useSelectedTargetId and the
SelectedTargetProvider shared state behavior. Cover URL parameter precedence
over stored selection, synchronization via the browser storage event across
tabs, and normalization of empty/invalid values to null. Place the tests near
the useSelectedTarget hook so regressions in this shared infrastructure are
caught without relying only on consumer component tests.
In `@schema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.tsx`:
- Around line 414-530: Replace the hand-rolled ZIP assembly in ZipWriter with a
well-tested client-side ZIP library such as fflate or client-zip. Keep the
ZipWriter API shape if needed, but move the binary header/CRC/EOCD logic out of
toBytes() and delegate archive creation to the library so future extensions like
compression and Zip64 are handled safely.
In `@src/main/scala/com/sslproxy/schema/cli/CliOpts.scala`:
- Around line 105-172: The Mongo config assembly in `serverOpts` currently uses
`(mongoUri, mongoDatabase, mongoTargetsCollection).mapN(MongoConfig.apply)`,
which turns any partially provided set of options into a generic `None`. Change
the validation around `mongoUriOpt`, `mongoDatabaseOpt`, and
`mongoTargetsCollectionOpt` so each field is checked independently and missing
values are reported by name when any Mongo setting is incomplete. Keep the final
`ServerConfig` wiring intact, but replace the silent `mapN` collapse with
explicit validation logic that preserves a clear error for the specific absent
option(s).
In `@src/main/scala/com/sslproxy/schema/server/DbPing.scala`:
- Line 12: Add unit tests around DbPing’s connectionError and redact helpers to
cover the credential-scrubbing behavior. Verify that exception messages
containing embedded database URLs like postgres://user:secret@host are redacted
before being returned, and that query parameters such as password= and pwd= are
stripped or masked. Use the existing DbPing/connectionError/redact flow so the
tests exercise the same safeguard used for client-facing connection-test errors.
In `@src/main/scala/com/sslproxy/schema/store/SqlFileStore.scala`:
- Around line 114-118: Remove the redundant secondary index creation in the
SqlFileStore.initialize method: keep the compound index on folder and filename,
but drop the separate Indexes.ascending("path") call because file.path is
already stored as _id in the same store and is implicitly indexed. Update the
initialize logic in SqlFileStore so it only creates the index that is still
needed.
- Around line 97-112: The `toSqlFiles` logic is duplicated in both
`SqlFileStore` implementations, so extract the shared conversion into a common
helper on the `SqlFileStore` trait using `list` as the source and remove the two
concrete `override def toSqlFiles` bodies. Keep the shared decoding and
`SqlFile` construction in one place so the Mongo and in-memory stores both reuse
the same implementation and cannot drift apart.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2803d98-7785-4155-9c0c-b25094d1df3f
⛔ Files ignored due to path filters (2)
schema-migrator-ui/bun.lockis excluded by!**/*.lockschema-migrator-ui/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (71)
.bsp/sbt.jsonAGENTS.mdREADME.mdbuild.sbtdocker-compose.ymlnginx/Dockerfilenginx/nginx.conf.templateschema-migrator-ui/package.jsonschema-migrator-ui/src/api/sqlFiles.tsschema-migrator-ui/src/components/AppBreadcrumbs.test.tsschema-migrator-ui/src/components/AppBreadcrumbs.tsxschema-migrator-ui/src/components/ConnectionForm.tsxschema-migrator-ui/src/components/DocumentTitle.test.tsschema-migrator-ui/src/components/DocumentTitle.tsxschema-migrator-ui/src/components/TargetSelector.tsxschema-migrator-ui/src/components/breadcrumbs.tsschema-migrator-ui/src/components/titleFormatting.tsschema-migrator-ui/src/components/ui/DataTable.tsxschema-migrator-ui/src/hooks/useSelectedTarget.tsxschema-migrator-ui/src/layouts/AppShell.tsxschema-migrator-ui/src/main.tsxschema-migrator-ui/src/pages/Drift/DriftPage.test.tsxschema-migrator-ui/src/pages/Drift/DriftPage.tsxschema-migrator-ui/src/pages/Patches/PatchListPage.tsxschema-migrator-ui/src/pages/Runs/RunListPage.tsxschema-migrator-ui/src/pages/Schema/SchemaPage.test.tsxschema-migrator-ui/src/pages/Schema/SchemaPage.tsxschema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.test.tsxschema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.tsxschema-migrator-ui/src/pages/Targets/TargetListPage.tsxschema-migrator-ui/src/router.tsxschema-migrator-ui/src/styles.cssschema-migrator-ui/src/test/render.tsxschema-migrator-ui/src/test/setup.tsschema-migrator-ui/src/types/index.tssrc/main/scala/com/sslproxy/schema/cli/CliOpts.scalasrc/main/scala/com/sslproxy/schema/cli/Commands.scalasrc/main/scala/com/sslproxy/schema/config/MigratorConfig.scalasrc/main/scala/com/sslproxy/schema/db/LockManager.scalasrc/main/scala/com/sslproxy/schema/db/postgres/PostgresProvider.scalasrc/main/scala/com/sslproxy/schema/discovery/DiscoveryService.scalasrc/main/scala/com/sslproxy/schema/discovery/SqlFile.scalasrc/main/scala/com/sslproxy/schema/engine/ManifestBuilder.scalasrc/main/scala/com/sslproxy/schema/engine/MigrationEngine.scalasrc/main/scala/com/sslproxy/schema/server/DbPing.scalasrc/main/scala/com/sslproxy/schema/server/HttpServer.scalasrc/main/scala/com/sslproxy/schema/server/JdbcConnectionProperties.scalasrc/main/scala/com/sslproxy/schema/server/LoggingMiddleware.scalasrc/main/scala/com/sslproxy/schema/server/PostgresDriftAnalyzer.scalasrc/main/scala/com/sslproxy/schema/server/Routes.scalasrc/main/scala/com/sslproxy/schema/server/SchemaRoutes.scalasrc/main/scala/com/sslproxy/schema/server/SqlFileRoutes.scalasrc/main/scala/com/sslproxy/schema/server/TargetRoutes.scalasrc/main/scala/com/sslproxy/schema/server/auth/JwtMiddleware.scalasrc/main/scala/com/sslproxy/schema/server/compress/Bzip2.scalasrc/main/scala/com/sslproxy/schema/server/compress/Bzip2Middleware.scalasrc/main/scala/com/sslproxy/schema/store/Models.scalasrc/main/scala/com/sslproxy/schema/store/MongoTargetStore.scalasrc/main/scala/com/sslproxy/schema/store/SqlFileStore.scalasrc/main/scala/com/sslproxy/schema/store/TargetStore.scalasrc/main/scala/com/sslproxy/schema/validation/DependencyValidator.scalasrc/main/scala/com/sslproxy/schema/validation/RollbackValidator.scalasrc/main/scala/com/sslproxy/schema/validation/Validator.scalasrc/test/scala/com/sslproxy/schema/config/ServerConfigSuite.scalasrc/test/scala/com/sslproxy/schema/db/ProviderSuite.scalasrc/test/scala/com/sslproxy/schema/discovery/DiscoveryServiceSuite.scalasrc/test/scala/com/sslproxy/schema/server/PostgresDriftAnalyzerSuite.scalasrc/test/scala/com/sslproxy/schema/server/RoutesSuite.scalasrc/test/scala/com/sslproxy/schema/server/auth/JwtMiddlewareSuite.scalasrc/test/scala/com/sslproxy/schema/server/compress/Bzip2Suite.scalasrc/test/scala/com/sslproxy/schema/store/TargetStoreSuite.scala
Comment on lines
+26
to
+36
| - BEDROCK_JWT_SECRET=${BEDROCK_JWT_SECRET:?BEDROCK_JWT_SECRET is required} | ||
| - BEDROCK_DEV_AUTH_SECRET=${BEDROCK_DEV_AUTH_SECRET:?BEDROCK_DEV_AUTH_SECRET is required} | ||
| - BEDROCK_API_BEARER_TOKEN=${BEDROCK_API_BEARER_TOKEN:?BEDROCK_API_BEARER_TOKEN is required} | ||
| - BEDROCK_MONGO_URI=${BEDROCK_MONGO_URI:-mongodb://mongo:27017} | ||
| - BEDROCK_MONGO_DATABASE=${BEDROCK_MONGO_DATABASE:-schema_migrator} | ||
| - BEDROCK_MONGO_TARGETS_COLLECTION=${BEDROCK_MONGO_TARGETS_COLLECTION:-targets} | ||
| - BEDROCK_DB_TEST_ALLOWED_HOSTS=${BEDROCK_DB_TEST_ALLOWED_HOSTS:-localhost,127.0.0.1,192.168.1.221} | ||
| - BEDROCK_DB_URL=${BEDROCK_DB_URL:?BEDROCK_DB_URL is required} | ||
| - BEDROCK_DB_USER=${BEDROCK_DB_USER:?BEDROCK_DB_USER is required} | ||
| - BEDROCK_DB_PASSWORD=${BEDROCK_DB_PASSWORD:?BEDROCK_DB_PASSWORD is required} | ||
| - DATABASE_URL=${BEDROCK_DB_URL:?BEDROCK_DB_URL is required} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Hardcoded private IP in shared default.
BEDROCK_DB_TEST_ALLOWED_HOSTS defaults to localhost,127.0.0.1,192.168.1.221. The specific 192.168.1.221 looks like a leftover developer/LAN IP baked into a committed default rather than a generic value, and will silently allowlist that host for anyone who doesn't override the variable.
💡 Suggested fix
- - BEDROCK_DB_TEST_ALLOWED_HOSTS=${BEDROCK_DB_TEST_ALLOWED_HOSTS:-localhost,127.0.0.1,192.168.1.221}
+ - BEDROCK_DB_TEST_ALLOWED_HOSTS=${BEDROCK_DB_TEST_ALLOWED_HOSTS:-localhost,127.0.0.1}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - BEDROCK_JWT_SECRET=${BEDROCK_JWT_SECRET:?BEDROCK_JWT_SECRET is required} | |
| - BEDROCK_DEV_AUTH_SECRET=${BEDROCK_DEV_AUTH_SECRET:?BEDROCK_DEV_AUTH_SECRET is required} | |
| - BEDROCK_API_BEARER_TOKEN=${BEDROCK_API_BEARER_TOKEN:?BEDROCK_API_BEARER_TOKEN is required} | |
| - BEDROCK_MONGO_URI=${BEDROCK_MONGO_URI:-mongodb://mongo:27017} | |
| - BEDROCK_MONGO_DATABASE=${BEDROCK_MONGO_DATABASE:-schema_migrator} | |
| - BEDROCK_MONGO_TARGETS_COLLECTION=${BEDROCK_MONGO_TARGETS_COLLECTION:-targets} | |
| - BEDROCK_DB_TEST_ALLOWED_HOSTS=${BEDROCK_DB_TEST_ALLOWED_HOSTS:-localhost,127.0.0.1,192.168.1.221} | |
| - BEDROCK_DB_URL=${BEDROCK_DB_URL:?BEDROCK_DB_URL is required} | |
| - BEDROCK_DB_USER=${BEDROCK_DB_USER:?BEDROCK_DB_USER is required} | |
| - BEDROCK_DB_PASSWORD=${BEDROCK_DB_PASSWORD:?BEDROCK_DB_PASSWORD is required} | |
| - DATABASE_URL=${BEDROCK_DB_URL:?BEDROCK_DB_URL is required} | |
| - BEDROCK_JWT_SECRET=${BEDROCK_JWT_SECRET:?BEDROCK_JWT_SECRET is required} | |
| - BEDROCK_DEV_AUTH_SECRET=${BEDROCK_DEV_AUTH_SECRET:?BEDROCK_DEV_AUTH_SECRET is required} | |
| - BEDROCK_API_BEARER_TOKEN=${BEDROCK_API_BEARER_TOKEN:?BEDROCK_API_BEARER_TOKEN is required} | |
| - BEDROCK_MONGO_URI=${BEDROCK_MONGO_URI:-mongodb://mongo:27017} | |
| - BEDROCK_MONGO_DATABASE=${BEDROCK_MONGO_DATABASE:-schema_migrator} | |
| - BEDROCK_MONGO_TARGETS_COLLECTION=${BEDROCK_MONGO_TARGETS_COLLECTION:-targets} | |
| - BEDROCK_DB_TEST_ALLOWED_HOSTS=${BEDROCK_DB_TEST_ALLOWED_HOSTS:-localhost,127.0.0.1} | |
| - BEDROCK_DB_URL=${BEDROCK_DB_URL:?BEDROCK_DB_URL is required} | |
| - BEDROCK_DB_USER=${BEDROCK_DB_USER:?BEDROCK_DB_USER is required} | |
| - BEDROCK_DB_PASSWORD=${BEDROCK_DB_PASSWORD:?BEDROCK_DB_PASSWORD is required} | |
| - DATABASE_URL=${BEDROCK_DB_URL:?BEDROCK_DB_URL is required} |
🤖 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 `@docker-compose.yml` around lines 26 - 36, The default value for
BEDROCK_DB_TEST_ALLOWED_HOSTS in docker-compose.yml includes a hardcoded private
LAN IP, which should not be committed as a shared default. Update the
environment entry to use only generic local test hosts, and if that host is
still needed, make it explicitly configurable via an override rather than baked
into the default. Keep the change focused on the BEDROCK_DB_TEST_ALLOWED_HOSTS
setting in the compose environment block.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
schema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.tsx (1)
411-431: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDrop the extra ZIP buffer copy
zipSyncalready returns aUint8Array, so this allocates and copies the whole archive again. Return thezipSyncresult directly, or cast it if you need the narrowerUint8Array<ArrayBuffer>annotation.🤖 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 `@schema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.tsx` around lines 411 - 431, The ZipWriter::toBytes method is making an unnecessary second copy of the ZIP archive after zipSync already returns the bytes. Update ZipWriter so toBytes returns the zipSync result directly, or adjust the return type annotation if needed, and keep toBlob using that single buffer path without reallocating or copying the archive data again.
🤖 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 `@src/main/scala/com/sslproxy/schema/store/MongoTargetStore.scala`:
- Around line 17-27: The MongoTargetStore.resource wiring currently accepts a
passwordKey that is reused from the HTTP response encryption key, so update the
call site and related key-loading logic to use a distinct key for target
password encryption. Introduce separate key material with clear domain
separation, such as a dedicated BEDROCK_TARGET_PASSWORD_KEY or an HKDF-derived
subkey, and keep the MongoTargetStore and PasswordCrypto usage unchanged except
for passing the new target-password-specific key. Identify the key plumbing in
HttpServer.scala and the MongoTargetStore.resource signature so the encryption
domains are no longer shared.
- Around line 57-70: `MongoTargetStore.update` currently does a non-atomic
read-then-write using `collection.find(...).first()` and
`collection.replaceOne(...)`, which can overwrite concurrent changes. Refactor
this path to use a single atomic MongoDB operation in `update` (prefer
`findOneAndReplace` or an equivalent atomic replace on the same `idFilter(id)`),
and build the replacement from the current persisted document returned by that
operation instead of a stale snapshot. Keep the existing helpers like
`storedFromDocument`, `documentFor`, and `toTarget` in place, but wire them
through the atomic call so the update logic in `MongoTargetStore` no longer
depends on separate read and write round-trips.
---
Nitpick comments:
In `@schema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.tsx`:
- Around line 411-431: The ZipWriter::toBytes method is making an unnecessary
second copy of the ZIP archive after zipSync already returns the bytes. Update
ZipWriter so toBytes returns the zipSync result directly, or adjust the return
type annotation if needed, and keep toBlob using that single buffer path without
reallocating or copying the archive data again.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e45a1b7-f1b0-4486-a12f-f00277fdce8d
⛔ Files ignored due to path filters (2)
schema-migrator-ui/bun.lockis excluded by!**/*.lockschema-migrator-ui/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
.gitignoreAGENTS.mdREADME.mdnginx/nginx.conf.templateschema-migrator-ui/package.jsonschema-migrator-ui/src/hooks/useSelectedTarget.test.tsxschema-migrator-ui/src/hooks/useSelectedTarget.tsxschema-migrator-ui/src/pages/Drift/DriftPage.test.tsxschema-migrator-ui/src/pages/Drift/DriftPage.tsxschema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.test.tsxschema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.tsxsrc/main/scala/com/sslproxy/schema/cli/CliOpts.scalasrc/main/scala/com/sslproxy/schema/config/MigratorConfig.scalasrc/main/scala/com/sslproxy/schema/db/postgres/PostgresProvider.scalasrc/main/scala/com/sslproxy/schema/server/DbPing.scalasrc/main/scala/com/sslproxy/schema/server/HttpServer.scalasrc/main/scala/com/sslproxy/schema/server/PostgresDriftAnalyzer.scalasrc/main/scala/com/sslproxy/schema/server/SchemaRoutes.scalasrc/main/scala/com/sslproxy/schema/server/SqlFileRoutes.scalasrc/main/scala/com/sslproxy/schema/server/compress/Bzip2Middleware.scalasrc/main/scala/com/sslproxy/schema/store/MongoTargetStore.scalasrc/main/scala/com/sslproxy/schema/store/SqlFileStore.scalasrc/main/scala/com/sslproxy/schema/store/TargetStore.scalasrc/test/scala/com/sslproxy/schema/config/ServerConfigSuite.scalasrc/test/scala/com/sslproxy/schema/db/ProviderSuite.scalasrc/test/scala/com/sslproxy/schema/server/DbPingSuite.scalasrc/test/scala/com/sslproxy/schema/server/PostgresDriftAnalyzerSuite.scalasrc/test/scala/com/sslproxy/schema/server/RoutesSuite.scalasrc/test/scala/com/sslproxy/schema/server/compress/Bzip2Suite.scalasrc/test/scala/com/sslproxy/schema/store/TargetStoreSuite.scala
✅ Files skipped from review due to trivial changes (3)
- .gitignore
- AGENTS.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (18)
- src/test/scala/com/sslproxy/schema/db/ProviderSuite.scala
- nginx/nginx.conf.template
- src/main/scala/com/sslproxy/schema/config/MigratorConfig.scala
- src/test/scala/com/sslproxy/schema/config/ServerConfigSuite.scala
- src/test/scala/com/sslproxy/schema/store/TargetStoreSuite.scala
- src/main/scala/com/sslproxy/schema/db/postgres/PostgresProvider.scala
- schema-migrator-ui/src/pages/SqlFiles/SqlFilesPage.test.tsx
- src/main/scala/com/sslproxy/schema/cli/CliOpts.scala
- src/main/scala/com/sslproxy/schema/server/SqlFileRoutes.scala
- src/main/scala/com/sslproxy/schema/server/DbPing.scala
- schema-migrator-ui/src/hooks/useSelectedTarget.tsx
- src/test/scala/com/sslproxy/schema/server/RoutesSuite.scala
- src/main/scala/com/sslproxy/schema/store/SqlFileStore.scala
- schema-migrator-ui/src/pages/Drift/DriftPage.test.tsx
- schema-migrator-ui/src/pages/Drift/DriftPage.tsx
- src/test/scala/com/sslproxy/schema/server/PostgresDriftAnalyzerSuite.scala
- src/main/scala/com/sslproxy/schema/server/SchemaRoutes.scala
- src/main/scala/com/sslproxy/schema/server/PostgresDriftAnalyzer.scala
Comment on lines
+17
to
+27
| object MongoTargetStore: | ||
| def resource(config: MongoConfig, passwordKey: SecretKeySpec): Resource[IO, TargetStore] = | ||
| Resource | ||
| .make(IO.blocking(MongoClients.create(config.uri)))(client => IO.blocking(client.close())) | ||
| .evalMap { client => | ||
| val store = MongoTargetStore( | ||
| client.getDatabase(config.database).getCollection(config.targetsCollection), | ||
| new PasswordCrypto(passwordKey) | ||
| ) | ||
| store.initialize.as(store: TargetStore) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Target password encryption key reuses the HTTP response encryption key.
Per HttpServer.scala's wiring (evidence provided), targetPasswordKey passed here is the same encryptKey used for AesGcmMiddleware response encryption. Sharing one AES-GCM key across two unrelated security domains (transient HTTP payload encryption vs. durable password-at-rest encryption) violates key-separation principles: a compromise or rotation need in one domain forces action in the other, and blast radius is unnecessarily widened.
Consider deriving a distinct key for target password encryption (e.g., a separate BEDROCK_TARGET_PASSWORD_KEY, or an HKDF-derived subkey from the master secret with a domain-separation label) rather than reusing BEDROCK_ENCRYPT_KEY for both purposes.
🤖 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 `@src/main/scala/com/sslproxy/schema/store/MongoTargetStore.scala` around lines
17 - 27, The MongoTargetStore.resource wiring currently accepts a passwordKey
that is reused from the HTTP response encryption key, so update the call site
and related key-loading logic to use a distinct key for target password
encryption. Introduce separate key material with clear domain separation, such
as a dedicated BEDROCK_TARGET_PASSWORD_KEY or an HKDF-derived subkey, and keep
the MongoTargetStore and PasswordCrypto usage unchanged except for passing the
new target-password-specific key. Identify the key plumbing in HttpServer.scala
and the MongoTargetStore.resource signature so the encryption domains are no
longer shared.
Comment on lines
+57
to
+70
| override def update(id: String, payload: TargetPayload): IO[Option[Target]] = | ||
| for | ||
| document <- IO.blocking(Option(collection.find(idFilter(id)).first())) | ||
| updated <- document.traverse { document => | ||
| for | ||
| existing <- storedFromDocument(document) | ||
| now <- nowString | ||
| target = toTarget(id, existing.target.created_at, payload) | ||
| password = payload.password.filter(_.nonEmpty).orElse(existing.password) | ||
| replacement <- documentFor(StoredTarget(target, password), now) | ||
| result <- IO.blocking(collection.replaceOne(idFilter(id), replacement)) | ||
| yield Option.when(result.getMatchedCount > 0)(target) | ||
| }.map(_.flatten) | ||
| yield updated |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Non-atomic read-then-write in update allows lost updates.
find().first() followed by a separate replaceOne(idFilter(id), replacement) is not atomic. A concurrent update to the same target between these two calls will be silently overwritten, since the replacement is built from a stale snapshot (existing.password, existing.target.created_at). The prior in-memory Ref-based store did not have this race.
💡 Suggested fix: use an atomic findOneAndReplace
override def update(id: String, payload: TargetPayload): IO[Option[Target]] =
for
document <- IO.blocking(Option(collection.find(idFilter(id)).first()))
updated <- document.traverse { document =>
for
existing <- storedFromDocument(document)
now <- nowString
target = toTarget(id, existing.target.created_at, payload)
password = payload.password.filter(_.nonEmpty).orElse(existing.password)
replacement <- documentFor(StoredTarget(target, password), now)
- result <- IO.blocking(collection.replaceOne(idFilter(id), replacement))
- yield Option.when(result.getMatchedCount > 0)(target)
+ replaced <- IO.blocking(Option(collection.findOneAndReplace(idFilter(id), replacement)))
+ yield replaced.as(target)
}.map(_.flatten)
yield updatedNote this collapses the write into one atomic round-trip; full protection against concurrent edits to the same target would additionally need optimistic concurrency (e.g., matching on an updated_at/version field in the filter).
🤖 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 `@src/main/scala/com/sslproxy/schema/store/MongoTargetStore.scala` around lines
57 - 70, `MongoTargetStore.update` currently does a non-atomic read-then-write
using `collection.find(...).first()` and `collection.replaceOne(...)`, which can
overwrite concurrent changes. Refactor this path to use a single atomic MongoDB
operation in `update` (prefer `findOneAndReplace` or an equivalent atomic
replace on the same `idFilter(id)`), and build the replacement from the current
persisted document returned by that operation instead of a stale snapshot. Keep
the existing helpers like `storedFromDocument`, `documentFor`, and `toTarget` in
place, but wire them through the atomic call so the update logic in
`MongoTargetStore` no longer depends on separate read and write round-trips.
This was referenced Jul 4, 2026
Merged
Merged
Merged
Merged
Merged
Merged
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
1 participant
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 by CodeRabbit