Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,24 @@ jobs:
# automatically by pip.
python -m pip install -e cli

# Build the Go wrapper from source so the linux-amd64 CI runner has an
# executable at wrappers/go/policy-wrapper. The committed binary (if any)
# may have been built on a different OS/arch (e.g. macOS arm64) and will
# raise OSError: [Errno 8] Exec format error when invoked.
- name: Set up Go
uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5
with:
go-version: "1.22"
cache: true
cache-dependency-path: wrappers/go/go.mod

- name: Build Go wrapper for the CI runner
run: |
set -euo pipefail
( cd wrappers/go && go build -o policy-wrapper ./... )
chmod +x wrappers/go/policy-wrapper
file wrappers/go/policy-wrapper

- name: Lint with ruff
run: ruff check .

Expand Down
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,10 @@ node_modules
# Setuptools / pip metadata (build artifacts)
*.egg-info/

# Go wrapper binary -- built from source (wrappers/go/main.go) per-CI runner
# because a committed prebuilt binary is wrong-arch on other platforms.
wrappers/go/policy-wrapper
wrappers/go/policy-wrapper.exe

# Runtime/utility scripts (not source)
ps_run.json
12 changes: 8 additions & 4 deletions tests/test_policy_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -633,20 +633,24 @@ def _validate_wrapper_bundle(
_validate_schema(payload, schema)

def _build_go_binary(self, binary: Path) -> bool:
if binary.exists():
return True

go_source = binary.parent
# Always rebuild from source when `go` is available. Trusting a
# pre-existing binary is unsafe because it may have been built on a
# different OS/arch (e.g. macOS arm64 committed into the repo) and
# would raise OSError: [Errno 8] Exec format error on linux-amd64
# CI runners. The Go module has no external deps, so this is fast.
Comment on lines +636 to +640

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add an FR trace reference (for example via # @trace FR-..., a @pytest.mark.requirement("FR-..."), or a docstring trace) in this modified test area so the file satisfies the repository's mandatory test-to-requirement traceability rule. [custom_rule]

Severity Level: Minor ⚠️

Why it matters? 🤔

The repository rule requires test files and new test functions to include an FR trace marker, requirement annotation, docstring trace, or FR-based test name. The modified test area shown here contains only explanatory comments and no FR-XXX-NNN reference or equivalent trace marker, so the violation is real.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** tests/test_policy_contract.py
**Line:** 636:640
**Comment:**
	*Custom Rule: Add an FR trace reference (for example via `# @trace FR-...`, a `@pytest.mark.requirement("FR-...")`, or a docstring trace) in this modified test area so the file satisfies the repository's mandatory test-to-requirement traceability rule.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

if shutil.which("go") is None:
return False
Comment on lines 641 to 642

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If go is not installed on the system/runner but a pre-existing binary already exists (for example, if it was built in a previous CI pipeline step and carried over to a minimal test container), returning False will unnecessarily skip or fail the tests. Since the binary is now git-ignored, any existing binary is highly likely to have been built locally for the correct architecture.

Consider returning binary.exists() when go is not available to allow using the pre-existing binary as a fallback.

Suggested change
if shutil.which("go") is None:
return False
if shutil.which("go") is None:
return binary.exists()


go_source = binary.parent
build = subprocess.run(
["go", "build", "-o", str(binary), "."],
cwd=str(go_source),
capture_output=True,
text=True,
)
if build.returncode != 0:
print(build.stdout)
print(build.stderr, file=__import__("sys").stderr)
Comment on lines +652 to +653

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since sys is already imported at the top of the file (line 10), you can reference sys.stderr directly instead of using __import__("sys").stderr.

Suggested change
print(build.stdout)
print(build.stderr, file=__import__("sys").stderr)
print(build.stdout)
print(build.stderr, file=sys.stderr)

return False
return binary.exists()

Expand Down
Binary file removed wrappers/go/policy-wrapper
Binary file not shown.
Loading