refactor(sync): align test harness with HLC-transport protocol#4
Merged
Conversation
Until now the vault-a/vault-b images cloned haex-vault from GitHub via HAEX_VAULT_REF, which made it impossible to E2E-test in-flight branches before they are pushed. Developers hit a cycle of push → trigger CI → wait even for trivial local iteration. Introduce a `local-vault` build context (Buildx named context) that defaults to a harmless dummy directory. When HAEX_VAULT_LOCAL_PATH and USE_LOCAL_VAULT=true are set in .env, the Dockerfile copies sources from the mount instead of cloning — node_modules, src-tauri/target, .nuxt and .output are stripped so host/container architecture mismatches cannot leak. All pnpm docker:* scripts now pass --env-file .env explicitly so the root-level .env is honoured (docker-compose would otherwise look beside the compose file in docker/).
The server no longer carries batch_id/batch_seq/batch_total fields (see haex-sync-server refactor/remove-batch-fields). Drop the entire batch-validation.spec.ts suite, the negative-batchSeq evil scenario, and the now-dead batch properties from the shared SyncChange helper and the crdt-behavior apply_remote_changes test payloads.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 57 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds optional local haex-vault build support via Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Docker as Docker Build
participant Local as Local FS (additional_context: local-vault)
participant GitHub as GitHub (haex-vault repo)
Dev->>Docker: docker compose build (--env-file .env)
Docker->>Docker: read ARG USE_LOCAL_VAULT
alt USE_LOCAL_VAULT == "true" and Local has src
Docker->>Local: bind-mount local-vault -> /tmp/local-vault
Docker->>Docker: copy /tmp/local-vault/* -> /repos/haex-vault
Docker->>Docker: remove node_modules, .nuxt, .output, src-tauri/target
else
Docker->>GitHub: git clone haex-vault @ HAEX_VAULT_REF -> /repos/haex-vault
end
Docker->>Docker: continue image build using /repos/haex-vault
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
7 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/Dockerfile`:
- Around line 30-55: The current COPY --from=local-vault . /tmp/local-vault/
creates a persistent layer that leaks sensitive files; remove that COPY and
change the following RUN block to use an ephemeral BuildKit bind mount (e.g. RUN
--mount=type=bind,from=local-vault,target=/tmp/local-vault ...) so the
local-vault contents are only mounted for that RUN and not committed to an image
layer; update the conditional that checks USE_LOCAL_VAULT and the paths
(/tmp/local-vault, /repos/haex-vault) accordingly and document that this
requires Docker BuildKit/experimental features to be enabled.
In `@package.json`:
- Around line 17-18: The package.json contains npm scripts that reference a
removed script (scripts/resolve-versions.sh) causing failures; remove or replace
the broken entries "docker:build:release", "docker:build:nightly",
"docker:test:release", "versions:resolve", "versions:release", and
"versions:nightly" from package.json and update README.md to remove any usage
examples calling scripts/resolve-versions.sh or the removed npm scripts, or
alternatively restore the missing scripts/resolve-versions.sh implementation and
ensure the script path and executable bits are correct so the listed npm scripts
work again; pick one approach (delete/references update OR restore the shell
script) and apply it consistently across package.json and documentation.
🪄 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 Plus
Run ID: e86afd6b-89d0-443c-ac6c-9224fe86ebec
📒 Files selected for processing (9)
.env.exampledocker/Dockerfiledocker/docker-compose.ymldocker/dummy-vault/.gitkeeppackage.jsontests/database/crdt-behavior.spec.tstests/helpers/sync-server-helpers.tstests/sync/batch-validation.spec.tstests/sync/evil-scenarios.spec.ts
💤 Files with no reviewable changes (4)
- tests/sync/evil-scenarios.spec.ts
- tests/helpers/sync-server-helpers.ts
- tests/database/crdt-behavior.spec.ts
- tests/sync/batch-validation.spec.ts
- Remove npm scripts referencing scripts/resolve-versions.sh and scripts/fetch-project-versions.sh (deleted in 0c729f7), along with orphan schemas/ and examples/ files. - Simplify README and .claude/architecture.md to match the current env-var-driven version config. - Replace Dockerfile `COPY --from=local-vault` with a BuildKit bind mount so local-vault contents never land in an image layer. Addresses CodeRabbit findings on PR #4.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/Dockerfile`:
- Around line 50-51: The shell commands use the unquoted variable
${HAEX_VAULT_REF} in the git clone --branch and git checkout invocations; wrap
the variable in double quotes (e.g., "--branch \"${HAEX_VAULT_REF}\"" and "git
checkout \"${HAEX_VAULT_REF}\"") so word-splitting and globbing cannot break the
git clone or checkout steps referenced in the Dockerfile's git clone lines and
the subsequent git checkout command.
🪄 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 Plus
Run ID: 24efcbd4-718f-4edc-8d96-7bbc96f1eef4
📒 Files selected for processing (6)
.claude/architecture.mdREADME.mddocker/Dockerfileexamples/.e2e-versions.example.jsonpackage.jsonschemas/e2e-versions.schema.json
💤 Files with no reviewable changes (2)
- schemas/e2e-versions.schema.json
- examples/.e2e-versions.example.json
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
Prevents word-splitting and globbing if the ref ever contains whitespace or shell metacharacters. Flagged by CodeRabbit on PR #4.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docker/Dockerfile (2)
55-59: Consider quotingHAEXTENSION_REFandVAULT_SDK_REFfor consistency.The fix for word-splitting was applied to
HAEX_VAULT_REFbut these refs remain unquoted. While they're unchanged lines and likely use simple refs, quoting them would provide consistent protection against edge cases with spaces or special characters.-RUN git clone --depth 1 --branch ${HAEXTENSION_REF} https://github.com/haex-space/haextension.git || \ - (git clone https://github.com/haex-space/haextension.git && cd haextension && git checkout ${HAEXTENSION_REF}) +RUN git clone --depth 1 --branch "${HAEXTENSION_REF}" https://github.com/haex-space/haextension.git || \ + (git clone https://github.com/haex-space/haextension.git && cd haextension && git checkout "${HAEXTENSION_REF}") -RUN git clone --depth 1 --branch ${VAULT_SDK_REF} https://github.com/haex-space/vault-sdk.git || \ - (git clone https://github.com/haex-space/vault-sdk.git && cd vault-sdk && git checkout ${VAULT_SDK_REF}) +RUN git clone --depth 1 --branch "${VAULT_SDK_REF}" https://github.com/haex-space/vault-sdk.git || \ + (git clone https://github.com/haex-space/vault-sdk.git && cd vault-sdk && git checkout "${VAULT_SDK_REF}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/Dockerfile` around lines 55 - 59, Quote the variable expansions for HAEXTENSION_REF and VAULT_SDK_REF in the Dockerfile git clone and checkout commands to prevent word-splitting and handle special characters; update the RUN lines that reference ${HAEXTENSION_REF} and ${VAULT_SDK_REF} (both in the --branch arguments and the subsequent git checkout uses) to use quoted expansions (e.g. "${HAEXTENSION_REF}" and "${VAULT_SDK_REF}").
11-11: Consider adding a non-root USER for production-like hardening.Trivy flags that no
USERcommand specifies a non-root user (DS-0002). For an E2E test environment with webtop, root may be intentional, but if the test workloads don't require elevated privileges, adding a non-root user would improve security posture.This can be deferred if webtop services require root or if this image is only used in isolated CI environments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/Dockerfile` at line 11, The Dockerfile currently uses the base image ghcr.io/haex-space/haex-e2e-base:latest without switching away from root; add a non-root user and switch to it to satisfy Trivy's DS-0002 check. Modify the Dockerfile to create a dedicated user (e.g., app or e2euser with a fixed UID/GID), ensure ownership/permissions on any needed directories (workdir, config, webtop data) are set, and add a USER <username> line before the image is finalized; if the base image already provides a non-root user, simply switch to that USER instead. Ensure any startup scripts or services referenced in the Dockerfile run correctly as the non-root user and document if root is required so this change can be deferred.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docker/Dockerfile`:
- Around line 55-59: Quote the variable expansions for HAEXTENSION_REF and
VAULT_SDK_REF in the Dockerfile git clone and checkout commands to prevent
word-splitting and handle special characters; update the RUN lines that
reference ${HAEXTENSION_REF} and ${VAULT_SDK_REF} (both in the --branch
arguments and the subsequent git checkout uses) to use quoted expansions (e.g.
"${HAEXTENSION_REF}" and "${VAULT_SDK_REF}").
- Line 11: The Dockerfile currently uses the base image
ghcr.io/haex-space/haex-e2e-base:latest without switching away from root; add a
non-root user and switch to it to satisfy Trivy's DS-0002 check. Modify the
Dockerfile to create a dedicated user (e.g., app or e2euser with a fixed
UID/GID), ensure ownership/permissions on any needed directories (workdir,
config, webtop data) are set, and add a USER <username> line before the image is
finalized; if the base image already provides a non-root user, simply switch to
that USER instead. Ensure any startup scripts or services referenced in the
Dockerfile run correctly as the non-root user and document if root is required
so this change can be deferred.
Same word-splitting/globbing risk as HAEX_VAULT_REF, fixed for consistency across all three ref-driven clones.
4 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
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
batch-validation.spec.tssuite, thenegative batchSeqevil scenario, and the dead batch properties from the sharedSyncChangehelper +crdt-behaviorapply-change payloadslocal-vaultBuildx named context sovault-a/vault-bcan build from a local haex-vault checkout instead of always cloning from GitHub (setHAEX_VAULT_LOCAL_PATH+USE_LOCAL_VAULT=truein.env). Stays opt-in; default behavior is unchanged.pnpm docker:*scripts now pass--env-file .envso the root-level.envis honoured (docker-compose looks beside the compose file by default).Why
Transaction-scope HLC is now the semantic grouping key on both haex-vault and haex-sync-server —
batchId/batchSeq/batchTotalhave been removed from the wire format. The E2E tests asserted behavior on fields that no longer exist, so three specs were either outdated or actively failing against the new server.The local-vault mount is a dev-ergonomics fix for the same refactor push: we couldn't E2E-test in-flight haex-vault branches without a CI roundtrip, which made iterating on the CRDT pipeline painful.
Companion work
Test plan
tests/extensions— 14/14 passtests/spaces— 167/167 pass (fixed:admin deletes the shared space,send invite to Personal space)tests/sync— 97/97 passSummary by CodeRabbit
New Features
Chores
Bug Fixes / Tests
Documentation