fix(#7298): pre-provision pre-commit hook envs and add golangci-lint to code sandbox - #7344
Open
SIDDHESH1564 wants to merge 3 commits into
Open
SIDDHESH1564 wants to merge 3 commits into
SIDDHESH1564 wants to merge 3 commits into
Conversation
…angci-lint to code sandbox The code/fix agent sandbox blocks outbound git-over-HTTPS at runtime (HTTP 403), so `pre-commit install-hooks` cannot clone the seven external hook repos declared in .pre-commit-config.yaml. Every affected agent run worked around this by manually invoking each hook's underlying tool individually — a list that silently drifts whenever .pre-commit-config.yaml changes. This commit addresses both root causes identified in fullsend-ai#7298: 1. Pre-provision pre-commit hook environments at image-build time. A new seed config (images/code/pre-commit-seed.yaml) lists the seven external repos at their pinned revs. During the code image build, `pre-commit install-hooks` clones each repo and builds its Python venv or Go binary into PRE_COMMIT_HOME=/sandbox/pre-commit-cache. At runtime the cache is already populated, so `pre-commit run` requires no outbound git fetch. The directory is sandbox-user-owned so the agent can extend it if hook revs change between image rebuilds. 2. Install golangci-lint into the code image. Agents trying to run `golangci-lint` at runtime hit the network block and timed out. golangci-lint is now installed via `go install` as the sandbox user (landing in /sandbox/go/bin, already on PATH) alongside the existing gopls install. Also adds two smoke-test assertions to sandbox-images.yml CI: golangci-lint must be at /sandbox/go/bin/golangci-lint, and PRE_COMMIT_HOME must be a non-empty directory. Updates docs/contributing/sandbox-topology.md to reflect the new tools. Signed-off-by: Siddhesh Khairnar <khairnarsiddhesh4057@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
1 similar comment
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
PR Summary by QodoPre-provision sandbox hooks and golangci-lint
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1. Code sandbox images fail to build
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+161
to
+163
| [ "$(command -v golangci-lint)" = "/sandbox/go/bin/golangci-lint" ] || { echo "FAIL: golangci-lint not at expected path ($(command -v golangci-lint))"; exit 1; } | ||
| [ -d "$PRE_COMMIT_HOME" ] || { echo "FAIL: PRE_COMMIT_HOME ($PRE_COMMIT_HOME) not a directory"; exit 1; } | ||
| [ -n "$(ls -A "$PRE_COMMIT_HOME" 2>/dev/null)" ] || { echo "FAIL: PRE_COMMIT_HOME ($PRE_COMMIT_HOME) is empty — hook pre-population failed"; exit 1; } |
There was a problem hiding this comment.
2. Sandbox workflow lacks guide review 📘 Rule violation § Compliance
sandbox-images.yml adds three smoke-test assertions, but neither those changes nor the PR description states that docs/contributing/ci-workflows.md was consulted. The omission is triggered by this workflow modification and leaves the new image-validation step without the explicit review evidence required for GitHub Actions changes.
Agent Prompt
## Issue description
The sandbox image workflow was modified without explicit evidence that `docs/contributing/ci-workflows.md` was consulted.
## Fix Focus Areas
- .github/workflows/sandbox-images.yml[161-163]
## Recommended Fix
Add an adjacent YAML comment stating that `docs/contributing/ci-workflows.md` was consulted and that this change only extends the existing smoke-test assertions without changing secrets, contexts, or workflow structure.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| && cd /tmp/pre-commit-seed-repo \ | ||
| && pre-commit install-hooks -c /tmp/pre-commit-seed.yaml \ | ||
| && cd / \ | ||
| && rm -rf /tmp/pre-commit-seed-repo /tmp/pre-commit-seed.yaml |
There was a problem hiding this comment.
1. Code sandbox images fail to build 🐞 Bug ≡ Correctness
COPY creates /tmp/pre-commit-seed.yaml as root, but the final pre-commit RUN executes after USER sandbox and tries to remove that file. With the inherited sticky /tmp, the non-owner unlink returns an error, so the final && rm -rf makes both architecture builds fail after hook installation.
Agent Prompt
## Issue description
The pre-commit seed is copied as root but removed by the sandbox user from sticky `/tmp`, causing the image build to fail during cleanup.
## Fix Focus Areas
- images/code/Containerfile[150-150]
- images/code/Containerfile[195-199]
## Recommended Fix
Copy the seed with `--chown=sandbox:sandbox` before switching users, or perform its cleanup in a root-owned build step. Preserve failure propagation from `pre-commit install-hooks` while ensuring the sandbox user can remove every temporary path.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+12
to
+13
| # Keep revs in sync with the root .pre-commit-config.yaml. | ||
| # A mismatch causes pre-commit to re-clone at runtime — defeating the purpose. |
There was a problem hiding this comment.
3. Hook updates leave agents offline 🐞 Bug ⚙ Maintainability
pre-commit-seed.yaml duplicates every external repository and revision from .pre-commit-config.yaml, but its only synchronization mechanism is a comment and the smoke test merely checks that the cache is non-empty. When an external hook is added or bumped only in the authoritative root configuration, the stale cache still passes validation and runtime pre-commit attempts the blocked Git clone for the absent entry.
Agent Prompt
## Issue description
The independently maintained seed can drift from the root pre-commit configuration without any CI or image smoke-test failure, restoring the runtime network failure this change is intended to eliminate.
## Fix Focus Areas
- images/code/pre-commit-seed.yaml[12-13]
- .pre-commit-config.yaml[3-77]
- .github/workflows/sandbox-images.yml[161-163]
## Recommended Fix
Add a checked script or test that parses both YAML files and fails unless every external repository, revision, and environment-affecting hook setting is represented in the seed. Run that check in normal CI or the image workflow so a root hook update cannot merge without the corresponding seed update.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
Pre-provisions pre-commit hook environments at image-build time and adds
golangci-lintto thefullsend-codesandbox, eliminating the recurring HTTP 403 failure that forced every code/fix agent run to manually re-implement hook checks.Related Issue
Closes #7298
Changes
images/code/Containerfile— setPRE_COMMIT_HOME=/sandbox/pre-commit-cache, create the directory (sandbox-user-owned),COPYthe seed config, installgolangci-lintviago install(as sandbox user, same pattern asgopls), runpre-commit install-hooksagainst the seed config in a throwaway git repo so all 7 external hook environments are baked in at build timeimages/code/pre-commit-seed.yaml— new seed config listing the 7 external repos at their pinned revs (matching.pre-commit-config.yaml); local hooks are excluded since they require the checked-out workspace.github/workflows/sandbox-images.yml— smoke test now assertsgolangci-lintis at/sandbox/go/bin/golangci-lintandPRE_COMMIT_HOMEis a non-empty directorydocs/contributing/sandbox-topology.md— updatefullsend-codekey-additions row to includegolangci-lintand the pre-commit hook cacheTesting
go vet ./...passes (no Go production code changed)sandbox-images.ymlandpre-commit-seed.yamlPRE_COMMIT_HOMEenv and directory creation run asrootbeforeUSER sandbox;golangci-lintinstall andpre-commit install-hooksrun assandboxuser after the Go toolchain is on PATHChecklist
fix(#7298):)Signed-off-by: Siddhesh Khairnar <khairnarsiddhesh4057@gmail.com>