Repository navigation
ACOMMONS-243 chore: Improve repository setup and agent instructions - #601
erwan-leforestier-sonarsource wants to merge 9 commits into
Conversation
hashicorp-vault-sonar-prod
Bot
changed the title
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
|
| "dockerfile": "Dockerfile" | ||
| }, | ||
| "remoteUser": "root", | ||
| "postCreateCommand": "mise trust mise.toml && mise install" |
There was a problem hiding this comment.
⚠️ Bug: Devcontainer now runs Maven automatically before Repox access exists
The devcontainer's postCreateCommand now runs mise install (.devcontainer/devcontainer.json:7). By the PR's own documentation (AGENTS.md:21, 30), the native postinstall hook then runs setup (mise.toml:16-17). setup calls mise doctor project (mise.toml:68), and that includes the maven:project check, which runs mvn -B -ntp -C -e -V ... -N validate (mise.toml:183-188).
So every new container runs Maven once it is created, before the user has had any chance to configure the approved Repox mirror and reader credentials inside it. This breaks the PR's stated constraints that the devcontainer "does not ... run Maven automatically" and that "approved Repox mirror access must be configured before Maven runs". It also contradicts README.md:48, which says to "Configure approved SonarSource Maven mirror and reader access before building". Parent resolution then goes to the default remote instead of the approved mirror.
The same hook fires in every CI and release job that uses jdx/mise-action, including the public github-ubuntu-latest-s publication jobs, before any Maven settings are in place.
Fix: keep the automatic hook from running the Maven probe. Either make maven:project an explicit or interactive check rather than part of the automatic post-install setup, or have the devcontainer only trust the config and leave the user to run mise install after configuring access.
Do not trigger the install/setup hook (and its Maven probe) automatically on container creation; tell the user to run it after configuring Repox access.:
"remoteUser": "root",
"postCreateCommand": "mise trust mise.toml && echo 'Configure Repox Maven access, then run: mise install'"
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| "local_checks": { | ||
| "committed_diff": ["git", "show", "--format=", "--check", "HEAD"], | ||
| "maven_verify": ["mvn", "clean", "verify", "-Pcoverage", "-DskipTests=false", "-B"] | ||
| }, |
There was a problem hiding this comment.
⚠️ Bug: Pre-push gate skips the Spotless check that fails the CI Build job
The push hook only runs git show --check and mvn clean verify -Pcoverage -DskipTests=false -B. Spotless is only in pluginManagement and has no execution bound to a lifecycle phase (pom.xml:163-180). CI runs it as a separate step, Check Java formatting: mise run lint (build.yml:60-61), followed by mise run check:source. So a commit with misformatted new or changed Java files passes the local gate, gets pushed, and then fails the required Build job. The gate exists to catch exactly that. The hook also calls a bare mvn from PATH and skips the repo's Mise entry points (vars.maven, Java 21 selection), even though AGENTS.md says to use Mise for repository commands. Add the lint step, and run the build through Mise, in local_checks. Then update both HOOKS.md files to match.
Mirror the CI Build job's checks through Mise:
"local_checks": {
"committed_diff": ["git", "show", "--format=", "--check", "HEAD"],
"maven_verify": ["mise", "run", "verify"],
"java_format": ["mise", "run", "lint"]
},
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| def git_push_tokens(tokens): | ||
| for index, token in enumerate(tokens): | ||
| if Path(token).name != "git": | ||
| continue | ||
| position = index + 1 | ||
| while position < len(tokens): | ||
| option = tokens[position] | ||
| if option in ("-c", "-C", "--git-dir", "--work-tree", "--namespace", "--config-env"): | ||
| position += 2 | ||
| elif option.startswith("-"): | ||
| position += 1 | ||
| else: | ||
| if option == "push": | ||
| return True | ||
| break |
There was a problem hiding this comment.
💡 Edge Case: Subshell, command-substitution and some bash -c pushes skip the gate
is_push splits the command on [;&| ] and shlex-tokenizes each segment. Then git_push_tokens requires a token whose basename is exactly git. For (git push) the tokens are (git, push). For echo $(git push) they are $(git, push). Neither matches, so pre_tool_use returns (True, "") and the push goes ahead with no checks. The bash -c unwrapping has a similar gap: it treats any option containing c as -c. With bash --norc -c 'git push', it inspects -c as the script, finds nothing, and breaks. The fallback scan then sees git push as a single token and also misses it. Both HOOKS.md files say shell-wrapped pushes are blocked. Strip leading (, $( and backticks from tokens, and only treat short-option clusters (-[a-z]*c[a-z]*, not --…) as -c.
Was this helpful? React with 👍 / 👎
| def authorized_override(reply, users): | ||
| latest = " ".join(users[-1].split()).casefold() if users else "" | ||
| if not latest.startswith("ci-override:"): | ||
| return None | ||
| for match in re.finditer(r'CI-OVERRIDE:\s*["\u201c](.+?)["\u201d]', reply): | ||
| quote = " ".join(match.group(1).split()).casefold() | ||
| if len(quote) >= 8 and quote == latest: | ||
| return match.group(1) | ||
| return None |
There was a problem hiding this comment.
💡 Edge Case: CI-OVERRIDE can never match a user message containing double quotes
authorized_override takes the quote from the reply with the non-greedy regex CI-OVERRIDE:\s*["“](.+?)["”], then requires it to equal the whole latest user message. Take a user who writes CI-OVERRIDE: ignore the "Promote npm" failure. The regex stops at the first inner ", so the captured text is shorter than the message and the override is always rejected, even when the assistant copies the message exactly as HOOKS.md asks. Anchor the match to the end of the line (e.g. CI-OVERRIDE:\s*["“](.+)["”]\s*$ with MULTILINE), or compare against the text after the opening quote.
Was this helpful? React with 👍 / 👎
| """Offline tests for the repository Codex hook contract.""" | ||
|
|
||
| import contextlib | ||
| import importlib.util | ||
| import io | ||
| import json | ||
| from pathlib import Path | ||
| import shutil | ||
| import subprocess | ||
| import sys | ||
| import tempfile | ||
| import unittest | ||
| from unittest import mock | ||
|
|
||
|
|
There was a problem hiding this comment.
💡 Quality: New hook unit tests are never run by CI or mise tasks
This commit adds .codex/test_checks.py and .claude/test_hooks.py, but nothing runs them. test:tooling, the only Python test entry point in CI (build.yml mise run test:tooling), runs scripts/test-mise.sh, and that only discovers scripts/test_packages.py. Nothing in mise.toml or the workflows mentions .codex or .claude. A regression in the push or stop logic would go unnoticed. Add python -m unittest discover -s .codex -p 'test_*.py' and the .claude equivalent to test:tooling.
Was this helpful? React with 👍 / 👎
| def stop_decision(payload, state): | ||
| """Return (block, text). text is the block reason or the plain status report.""" | ||
| report = ("Complete: " if state["complete"] else "Incomplete: ") + state["message"] | ||
| if state["complete"] or state["pr_error"]: | ||
| # No PR for this exact HEAD: CI is not in play for this turn, so only report. | ||
| return False, report | ||
| actions = {} | ||
| for kind, name, status in state["items"]: | ||
| actions.setdefault(classify(kind, status), []).append(f"{name}: {status}") | ||
| fix, monitor = actions.get("fix", []), actions.get("monitor", []) | ||
| if not fix and not monitor: | ||
| return False, report | ||
| reply = payload.get("last_assistant_message") or "" | ||
| if not fix and WAITING_MARKER in reply: | ||
| return False, f"CI still running; monitoring declared.\n{report}" |
There was a problem hiding this comment.
⚠️ Security: Stop hook tells agents to commit and push in sessions not asked to edit
The Stop hook doesn't know what the user asked for. collect_status() only checks whether the current HEAD has a PR (.codex/checks.py:389-406), and stop_decision blocks whenever any item counts as fix (:445-466). The block reason tells the agent to "investigate logs, fix, commit, and push; rerun a job with gh" (:458). Both agents register this for every Stop event: .codex/hooks.json:4, and .claude/settings.json:11-15 through .claude/hooks.py:79-91.
The fix set covers failed PR jobs. It also covers missing or stale local results (append_local_item → "failed (stale or missing results)", classified as fix at :419), which happens whenever the temp-dir state is gone. So a user can check out a branch whose PR has a red job, or just has no saved results, and ask a read-only question (explain this code, review this PR). The agent's turn is still forcibly continued with instructions to change code, commit, push to the remote, and rerun CI jobs. The user never authorized any of that. The loop guard only lets the stop through after three identical blocks, so the instruction is delivered first.
Fix: only block and give write or remediation instructions when this session pushed HEAD. For example, record the session id or pushed SHA in the pre-tool-use hook when a push passes, and require it in stop_decision. In every other case, return the status report as a plain systemMessage.
Was this helpful? React with 👍 / 👎
Code Review
|
| Auto-apply | Compact | Unblock |
|
|
|
Was this helpful? React with 👍 / 👎 | Gitar
Summary
SARA found missing module guidance, formatting, and local verification instructions. This change adds agent instructions, direct Maven commands, and the shared SonarSource Java formatter.
Changes
Document all eight modules, source and fixture boundaries, API contracts, exporter behavior, and tested native Maven commands.
Add native Spotless apply/check goals with Eclipse and the SonarSource formatter profile. Sonar Java recommends this profile, and Sonar PHP uses Spotless with the same Eclipse style. The plugin loads the canonical HTTPS URL directly.
Run Spotless check in the existing Build job after shared Maven setup/build. Full checkout history and
origin/masterselect whole Java files added or modified since the common ancestor. Format errors fail CI. Main/test Java is included; resources and generated output are excluded. No existing source is bulk-formatted.Remove all custom Python checks, their tests and CI workflow, the PR-title/size gates, and the container Python requirement. Keep the existing organization SCA check and NEXT quality gate, Maven coverage/PMD/SBOM, and the separate Git source-drift check. Remove the added GitHub dependency-review workflow because this repository does not support its required Dependency graph service. Preserve existing version selection and CODEOWNERS. No local commit hook is added.
Require approved Repox Maven settings before local or container Maven commands. Container creation runs no automatic Maven command. Use
sonar-xsfor internal release promotions andgithub-ubuntu-latest-sfor public NuGet/npm publication. No release or publication was run for validation.Validation and limits
mvn -B -ntp -C -Pcoverage clean verifypassed all 1,626 Java tests and coverage checks, with no failures, errors, or skipped tests.queuekey message excluded. The container built and reported Maven/JDK versions with network disabled. The established EditorConfig parser confirmed that all 111 resources keep encoding, line ending, and whitespace settings unchanged; file hashes are unchanged.fe548204passed Build and formatting and the organization-required SCA check. NEXT PR 601 returned an OK quality gate with new malware, licensing, and vulnerability conditions. The removed duplicate workflow failed because GitHub Dependency graph is unavailable;@v5itself resolves a valid branch. No repository service settings or version policy changed. The hosted Build job and formatter check passed for commit9060f06d.SARA finding-to-change mapping
Each required SARA finding has one row, including repeated observations. Finding text is quoted from
sara-findings.csv. The ID column showsfinding_order / original_evidence_order. The result and how-addressed columns state the actual change and remaining limits. File links open the current PR diff.60 required findings: 36 addressed, 15 partial, 8 excluded by user request, 1 not applicable.
No AGENTS.md, CLAUDE.md, .cursorrules or .github/copilot-instructions.md was found inside the repository; the only operational content is the 'mvn clean install' line in README.md.
CLAUDE.md
.github/copilot-instructions.md
No ADRs, architecture or design documents, or a CHANGELOG were found in the explored files.
commons/AGENTS.md
regex-parsing/AGENTS.md
xml-parsing/AGENTS.md
sonar-analyzer-commons-configurations/AGENTS.md
WARNING: No marked frozen or generated zones were found in the repository. README.md has no mention of generated or do-not-edit paths, and no CI drift or clean-tree check was detected.
.github/workflows/build.yml
No secret scanning configuration (pre-commit hook, CI scan step, or baseline file) was detected in the explored files, so leaks would not be blocked at commit or merge time.
No CONTRIBUTING guide, AGENTS.md or CLAUDE.md was found in the explored root files, so commit message format and branch naming are not documented.
No conventions, test commands for a single module, or common pitfalls are documented for agents.
The test-commons verifier corpus and the NuGet and npm packaging assets under sonar-analyzer-commons-configurations are not flagged as sensitive in any agent-facing file within the repo.
test-commons/AGENTS.md
sonar-analyzer-commons-configurations/AGENTS.md
No byte-for-byte reproducibility check or content-addressed lock was found in explored files, and the version is a SNAPSHOT (2.36-SNAPSHOT) for local builds (static estimate only).
No build cache configuration or documented build duration was found, and
mvn clean installalways starts from a clean state across eight modules.AGENTS.md
No lockfile or checksum-verified resolution covering transitive dependencies was found, so transitive versions depend on upstream POMs.
.github/workflows/build.yml
The npm and NuGet packages assembled in the configurations module are built by CI with no lockfile seen at the root.
No architecture or layering tests were found to enforce this at merge time.
No document designating a reference module as the model for new code was found, and no AGENTS.md or contributing guide was found at the root.
recognizers/AGENTS.md
regex-parsing/AGENTS.md
No module-info.java or tooling that enforces the public API surface was found, so visibility rules rest on convention.
No .editorconfig, formatter configuration or pre-commit configuration was found at the repository root, so formatting is not auto-fixed locally.
.editorconfig
AGENTS.md
README.md
.github/workflows/build.yml
Style enforcement appears to depend on the inherited SonarSource parent pom and CI analysis rather than on a commit-time hook.
No cycle or import-rule enforcement, such as architecture tests, was found, so module boundaries rely on convention.
No naming-convention enforcement configuration was found in the repository's own files, so conventions rest on review habit.
pom.xml
AGENTS.md
No code generators, templates or archetypes that guarantee this regularity were found in the explored files.
recognizers/AGENTS.md
regex-parsing/AGENTS.md
A full multi-module Maven install with a coverage profile in CI is likely to take more than 2 minutes cold, and no faster local path is documented.
README.md
No dependency lockfile, pinned container image, or exact toolchain version was found, so the resolved environment can drift between runs.
.devcontainer/devcontainer.json
AGENTS.md
Recreation steps are documented only as
mvn clean install, with no instructions for recreating the toolchain.AGENTS.md
.devcontainer/devcontainer.json
No documented command for building or testing a single module (for example with Maven's
-pland-amoptions) was found, and the README mentions only the full build.regex-parsing/AGENTS.md
Several modules (performance-measure, sonar-analyzer-commons-configurations) are missing from the README module list.
README.md does not mention mise, the required JDK version (21), or the Maven version, so a developer must discover these prerequisites on their own.
AGENTS.md
No bootstrap script, Makefile, or Maven wrapper was found in the explored root files.
AGENTS.md
Without agent instructions or architecture notes, an agent likely opens 5 to 10 files (module pom.xml, package-info files, sibling classes and tests) to learn conventions before editing.
commons/AGENTS.md
regex-parsing/AGENTS.md
README.md does not state the required JDK version, how to run tests for a single module, or whether private artifact repositories need credentials.
AGENTS.md
.devcontainer/devcontainer.json
The root README module list is out of date against pom.xml, missing two declared modules.
Module READMEs state 'Copyright 2009-2023' and commons/README.md says 'compatible with SQ >=7.9', which look stale against a 2.36-SNAPSHOT version dated 2026.
xml-parsing/README.md
regex-parsing/README.md
CODEOWNERS is a single catch-all line, so shared parsing code and publishing logic have no distinct owners or stricter review gates.
No repository-level guidance tells an agent which operations (releases, tagging, promotion, force pushes) require explicit human authorization. The build job holds contents: write.
sonar-analyzer-commons-configurations/AGENTS.md
No repository-level AGENTS.md or CLAUDE.md was found in the explored root files. The workspace-level AGENTS.md sits outside the repo and does not enumerate allowed or forbidden tools and network access for this codebase.
.devcontainer/devcontainer.json
.github/workflows/release.yml
No blocking vulnerability scanning of dependencies on pull requests and no SBOM generation were detected in the explored workflows or pom.xml.
AGENTS.md
No per-PR flake-detection check or quarantine convention was found in the explored workflows.
No documented scaffold or generator for new tests, and no AGENTS.md or contributor guidance on writing tests, was found.
test-commons/AGENTS.md
regex-parsing/AGENTS.md
No enforced coverage threshold or mutation testing configuration was found in the root pom.xml or the build workflow.
AGENTS.md
No documentation of scoped test selection or any change-to-test mapping was found.
No documented change-impact or affected-module command was found in the README or CI, so the agent must discover the module-scoped workflow itself.
The README does not mention the toolchain setup file, test-only or coverage profiles (CI uses
-Pcoverage), or the extra modules (configurations, performance-measure) that are in the build.AGENTS.md
The Maven pin is a floating minor (3.9), no Maven wrapper was found, and the README does not mention mise, so local setup relies on the developer knowing to use it.
No doctests or inline name-versus-behavior checks run during development; verification happens in the CI Maven build.
AGENTS.md
No inline dead-code detection was found in the explored files.
pom.xml
AGENTS.md
.vscode/extensions.json
Duplication feedback is not available in the editor or at commit time.
AGENTS.md
No local pre-commit or editor-level type or null-checking configuration was found; compile checks run through the CI Maven build (static estimate only).
.vscode/extensions.json
No local or in-editor size feedback was found in the explored files.
AGENTS.md
The mise.toml versions are floating minors (21.0, 3.9) and the file is not referenced from the README. No container or devcontainer spec was found.
.devcontainer/Dockerfile
AGENTS.md
No separate post-setup healthcheck or environment diagnosis script was found.
README.md
.devcontainer/devcontainer.json
The root module list omits sonar-analyzer-commons-configurations and performance-measure, which the root pom.xml declares as modules.
recognizers/README.md is a single descriptive sentence and gives no map of its classes.
recognizers/AGENTS.md
No sandboxed or containerized dev environment configuration was found.
.devcontainer/devcontainer.json
AGENTS.md
The documented command is a full clean install rather than an incremental verify-only command. Static estimate only.
AGENTS.md
.github/workflows/build.yml
The documented command is
clean installacross every module, and no faster development loop is described.README.md
No documented fast-path subset (lint-only or unit-only) was found, and actual run duration cannot be determined statically (static estimate only).
.github/workflows/build.yml
The README only documents a basic build command (mvn clean install) and does not describe contribution rules.
No CI check enforcing commit or PR title format was detected in the explored workflows (static estimate only).
No CI gate evaluating PR size was detected in the explored workflows.
No documented decomposition guidance for changes was found.
The branching strategy is visible only in workflow files and is not documented in a README or contribution guide.
CI feedback is delivered through hosted build and analysis tooling; no explicit statement in agent docs that CI must pass before merge was found.
Summary by Gitar
release.ymlto usesonar-xsandgithub-ubuntu-latest-srunners.github/workflows/dependency-review.ymlThis will update automatically on new commits.