fix: initialize git submodules and freeze the lockfile in the web session setup hook - #10348
fix: initialize git submodules and freeze the lockfile in the web session setup hook#10348dgarros wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
2 issues found across 1 file
Confidence score: 3/5
- In
dev/setup-environment.sh, a successfuluv synccan still leaveinfrahub_sdkunimportable while setup reports success, causing downstream failures that are harder to diagnose — re-run the import check after rebuilding and fail setup if it still fails. dev/setup-environment.shhas no automated regression coverage for the new manifest-failure and poisoned-venv rebuild paths, so future changes could silently break recovery behavior — add focused tests for both paths.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="dev/setup-environment.sh">
<violation number="1" location="dev/setup-environment.sh:98">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
This behavior change has no automated regression test: the new manifest failure path and poisoned-venv rebuild path rely only on manual verification. Add a focused shell/Bats test that asserts missing manifests fail and the recovery path is invoked.</violation>
<violation number="2" location="dev/setup-environment.sh:120">
P1: When the initial import check fails, this rebuild is the only subsequent validation; a successful `uv sync` can still leave `infrahub_sdk` unimportable, after which setup reports success. Re-run the import check after rebuilding and exit nonzero if it still fails.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| # Force the rebuild when the import is broken. | ||
| if ! uv run --no-sync python -c "import infrahub_sdk" &> /dev/null; then | ||
| echo "infrahub_sdk is not importable, rebuilding infrahub-server..." | ||
| uv sync --all-groups --reinstall-package infrahub-server |
There was a problem hiding this comment.
P1: When the initial import check fails, this rebuild is the only subsequent validation; a successful uv sync can still leave infrahub_sdk unimportable, after which setup reports success. Re-run the import check after rebuilding and exit nonzero if it still fails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/setup-environment.sh, line 120:
<comment>When the initial import check fails, this rebuild is the only subsequent validation; a successful `uv sync` can still leave `infrahub_sdk` unimportable, after which setup reports success. Re-run the import check after rebuilding and exit nonzero if it still fails.</comment>
<file context>
@@ -73,13 +73,53 @@ echo "Project directory: $PROJECT_DIR"
+ # Force the rebuild when the import is broken.
+ if ! uv run --no-sync python -c "import infrahub_sdk" &> /dev/null; then
+ echo "infrahub_sdk is not importable, rebuilding infrahub-server..."
+ uv sync --all-groups --reinstall-package infrahub-server
+ fi
+
</file context>
| uv sync --all-groups --reinstall-package infrahub-server | |
| uv sync --all-groups --reinstall-package infrahub-server | |
| if ! uv run --no-sync python -c "import infrahub_sdk" &> /dev/null; then | |
| echo "Error: infrahub_sdk is still not importable after rebuilding infrahub-server" >&2 | |
| exit 1 | |
| fi |
| exit 1 | ||
| fi | ||
|
|
||
| for manifest in "python_sdk/pyproject.toml" "frontend/packages/schema-visualizer/package.json"; do |
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
This behavior change has no automated regression test: the new manifest failure path and poisoned-venv rebuild path rely only on manual verification. Add a focused shell/Bats test that asserts missing manifests fail and the recovery path is invoked.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/setup-environment.sh, line 98:
<comment>This behavior change has no automated regression test: the new manifest failure path and poisoned-venv rebuild path rely only on manual verification. Add a focused shell/Bats test that asserts missing manifests fail and the recovery path is invoked.</comment>
<file context>
@@ -73,13 +73,53 @@ echo "Project directory: $PROJECT_DIR"
+ exit 1
+fi
+
+for manifest in "python_sdk/pyproject.toml" "frontend/packages/schema-visualizer/package.json"; do
+ if [ ! -f "$PROJECT_DIR/$manifest" ]; then
+ echo "Error: $manifest is missing after submodule initialization" >&2
</file context>
Claude Code on the web clones the repository without --recurse-submodules, so
python_sdk and frontend/packages/schema-visualizer are empty when the
SessionStart hook runs dev/setup-environment.sh. Both are consumed as local
paths rather than published packages, and neither installer treats an empty
directory as an error:
- python_sdk/infrahub_sdk is vendored into the infrahub-server distribution
via [tool.hatch.build.targets.wheel.sources], so uv builds a wheel with no
infrahub_sdk in it. `import infrahub` works while `import infrahub_sdk`
raises ModuleNotFoundError, breaking the 317 backend modules that use it.
- frontend/packages/schema-visualizer is a pnpm workspace package, which pnpm
resolves to a link with no dependency graph.
Both failures are silent, so the session looks correctly provisioned. Initialize
the submodules before the installers run and verify the expected manifests
exist, so a partial fetch stops setup instead of degrading it.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ppx4cTBD3xq76fvRu1joqm
4afb42e to
41cf861
Compare
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 3/5
dev/setup-environment.shcan swallow apnpm installfailure and still report success, leaving development environments partially broken and making diagnosis harder; propagate the install failure or verify the command result before printing success.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="dev/setup-environment.sh">
<violation number="1" location="dev/setup-environment.sh:118">
P2: This swallows a pnpm install failure and then unconditionally prints "Frontend dependencies installed", so a failed frontend install leaves the environment partially broken while the script reports success. The PR's stated goal is to "fail clearly" and "prevent silent, partially-working environments," but this change does the opposite: with `set -e` active, `cmd || echo ... (continuing)` makes the compound command exit 0, then execution continues and `exit 0` reports success. `--frozen-lockfile` makes failure more likely (any lockfile drift aborts), so the failure path is not rare. This is also a regression from the prior `pnpm install` line, which would abort on failure. Drop the `|| echo ...(continuing)` guard (and the unconditional success echo) so a frontend install failure aborts setup, consistent with the submodule manifest guard which exits 1.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| pnpm install --frozen-lockfile || echo "Warning: pnpm install failed (continuing)" | ||
| echo "Frontend dependencies installed" |
There was a problem hiding this comment.
P2: This swallows a pnpm install failure and then unconditionally prints "Frontend dependencies installed", so a failed frontend install leaves the environment partially broken while the script reports success. The PR's stated goal is to "fail clearly" and "prevent silent, partially-working environments," but this change does the opposite: with set -e active, cmd || echo ... (continuing) makes the compound command exit 0, then execution continues and exit 0 reports success. --frozen-lockfile makes failure more likely (any lockfile drift aborts), so the failure path is not rare. This is also a regression from the prior pnpm install line, which would abort on failure. Drop the || echo ...(continuing) guard (and the unconditional success echo) so a frontend install failure aborts setup, consistent with the submodule manifest guard which exits 1.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/setup-environment.sh, line 118:
<comment>This swallows a pnpm install failure and then unconditionally prints "Frontend dependencies installed", so a failed frontend install leaves the environment partially broken while the script reports success. The PR's stated goal is to "fail clearly" and "prevent silent, partially-working environments," but this change does the opposite: with `set -e` active, `cmd || echo ... (continuing)` makes the compound command exit 0, then execution continues and `exit 0` reports success. `--frozen-lockfile` makes failure more likely (any lockfile drift aborts), so the failure path is not rare. This is also a regression from the prior `pnpm install` line, which would abort on failure. Drop the `|| echo ...(continuing)` guard (and the unconditional success echo) so a frontend install failure aborts setup, consistent with the submodule manifest guard which exits 1.</comment>
<file context>
@@ -115,7 +115,7 @@ echo "Installing frontend dependencies with pnpm..."
if [ -d "$PROJECT_DIR/frontend/app" ]; then
cd "$PROJECT_DIR/frontend/app"
- pnpm install
+ pnpm install --frozen-lockfile || echo "Warning: pnpm install failed (continuing)"
echo "Frontend dependencies installed"
cd "$PROJECT_DIR"
</file context>
| pnpm install --frozen-lockfile || echo "Warning: pnpm install failed (continuing)" | |
| echo "Frontend dependencies installed" | |
| pnpm install --frozen-lockfile | |
| echo "Frontend dependencies installed" |
| exit 1 | ||
| fi | ||
|
|
||
| for manifest in "python_sdk/pyproject.toml" "frontend/packages/schema-visualizer/package.json"; do |
There was a problem hiding this comment.
is this like a healthcheck, that submodules were cloned successfully?
There was a problem hiding this comment.
Yes exactly .. not sure if this is really required .. Claude came up with it
There was a problem hiding this comment.
Something with git submodule status?
Why
Two independent defects in
dev/setup-environment.sh, theSessionStarthook for Claude Code on the web. One is new, one is a backport of a fix already ondevelop.1. Submodules are never initialized. The web container clones without
--recurse-submodules, sopython_sdkandfrontend/packages/schema-visualizerare empty when the hook runs. Both are consumed as local paths rather than published packages, and neither installer treats an empty directory as an error — so setup reports success while producing a broken environment.2. The lockfile churns on every session. A plain
pnpm installre-resolves the caret ranges inpackage.jsonand picks up whatever transitive versions have been published since the lockfile was committed, rewritingpnpm-lock.yaml. Observed on a real session:oxlint1.74→1.77,@oxc-parser0.141→0.143, a new@astrojs/compiler— a 780-line diff nobody asked for. This was fixed ondevelopin #9972 but never reachedstable.Goal: every web session starts with both submodules populated and a clean working tree, or setup stops with a clear error.
Non-goals: no change to how submodules are pinned or updated, and no change to the Python install command.
What changed
Two commits against
stable:fix: initialize git submodules in the web session setup hook(new) — runsgit submodule update --init --recursiveabove the Python and frontend installers, then asserts the two expected manifests exist so a partial fetch stops setup instead of silently degrading it. The command is already the canonical one documented indev/guidelines/git-workflow.md.fix(setup): use --frozen-lockfile in setup hook to stop lockfile churn (#9972)— cherry-picked fromdevelop(65efe7bf, provenance recorded viacherry-pick -x). One line, unmodified from the original.Net: +23 / -1 in one file. No dependency, lockfile, schema, or CI changes. The script still runs exclusively in the
claude-code-webenvironment — the existingSUPPORTED_ENVIRONMENTSgate is untouched, so local and CI runs exit early exactly as before.uv.lockandpnpm-lock.yamlare unmodified.Why the submodule failures are silent
python_sdk/infrahub_sdkis vendored into theinfrahub-serverdistribution via[tool.hatch.build.targets.wheel.sources], not installed as a separate dependency. With the directory empty,uv syncsucceeds and builds a wheel containing noinfrahub_sdk—import infrahubworks whileimport infrahub_sdkraisesModuleNotFoundError. 317 backend modules importinfrahub_sdk, so anything touching the SDK was broken in every web session.frontend/packages/schema-visualizeris a pnpm workspace package; with the directory empty pnpm resolves it to a link with no dependency graph and installs 43 fewer packages.How to review
The submodule block is ordering-sensitive — it must stay above the Python section, which is the entire point of that commit. The second commit is a verbatim cherry-pick; reviewing it against #9972 is enough.
How to test
Observed on this branch with both commits applied: exit 0, both submodules checked out at the pinned commits, all three imports succeed, pnpm reports all 5 workspace projects in scope, and the working tree stays clean. Re-running is idempotent. The manifest guard was exercised separately against an empty directory and exits 1 with the expected message.
shellcheckon the file reports only a pre-existingSC2064at theghinstall trap, which this PR leaves alone.Impact & rollout
submodules: true, so CI behavior is unchanged.--frozen-lockfilemakes the frontend install faster by skipping resolution.Checklist
dev/guidelines/git-workflow.mdalready documents the submodule command, and the change makes the setup script follow it.