-
Notifications
You must be signed in to change notification settings - Fork 61
[AI] OSAC-1316: osac get publicips fails: CEL references removed compute_instance field #676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
osac-jira-ai-issue-solver
wants to merge
4
commits into
main
Choose a base branch
from
osac-jira-ai-issue-solver/OSAC-1316
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
78e78f9
OSAC-1316: osac get publicips fails: CEL references removed compute_i…
osac-jira-ai-issue-solver[bot] c3cea47
OSAC-1316: address PR feedback
osac-jira-ai-issue-solver[bot] 0b9fbb5
OSAC-1316: address PR feedback
osac-jira-ai-issue-solver[bot] 48a43fa
OSAC-1316: address PR feedback
osac-jira-ai-issue-solver[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # .ai-bot/config.yaml — fulfillment-service | ||
| # | ||
| # Per-repository configuration for the jira-ai-issue-solver bot. | ||
| # See repoconfig/config.go for the schema. | ||
|
|
||
| # Validation commands the AI can use. These are hints — the AI decides | ||
| # when and how to run them. Listed in recommended execution order. | ||
| validation_commands: | ||
| - "buf lint" | ||
| - "buf generate" | ||
| - "go generate ./..." | ||
| - "gofmt -s -w ." | ||
| - "go build ./cmd/fulfillment-service" | ||
| - "go build ./cmd/osac" | ||
| - "ginkgo run -r internal" | ||
|
|
||
| # Auxiliary repositories cloned into the workspace before AI execution. | ||
| imports: | ||
| - repo: https://github.com/flightctl/ai-workflows | ||
| path: .ai-workflows | ||
| ref: main | ||
| install: .ai-workflows/install.sh claude | ||
| excludes: | ||
| - .artifacts/ | ||
|
|
||
| # Pull request settings. | ||
| pr: | ||
| draft: true | ||
| title_prefix: "[AI]" | ||
| labels: | ||
| - ai-pr | ||
|
|
||
| # AI provider settings. | ||
| ai: | ||
| claude: | ||
| allowed_tools: "Bash Edit Read Write" |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "image": "quay.io/rh-ee-andalton/osac-ai:latest" | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # Feedback Workflow | ||
|
|
||
| Read and execute `.ai-workflows/bugfix/skills/feedback.md`. | ||
|
|
||
| All artifact paths (`.artifacts/bugfix/{issue}/`) should use `.ai-bot/` | ||
| instead. | ||
|
|
||
| Review comments are already provided in the task above (use source 1: task | ||
| file). | ||
|
|
||
| ## Session Context Recovery | ||
|
|
||
| Before making changes, read `.ai-bot/session-context.md` if it exists. This | ||
| file contains the reasoning behind the original implementation (root cause, | ||
| design decisions, test strategy). Use it to avoid contradicting prior decisions | ||
| unless the reviewer explicitly asks for a different approach. | ||
|
|
||
| ## Addressing Comments | ||
|
|
||
| For each review comment: | ||
|
|
||
| 1. Read the comment carefully. Understand what is being asked. | ||
| 2. If the comment requests a code change, implement it. | ||
| 3. If the comment asks a question, answer it in `.ai-bot/comment-responses.json` | ||
| and make any related code changes. | ||
| 4. If you disagree with a suggestion, explain why in the comment response but | ||
| still implement it unless doing so would introduce a correctness bug. | ||
|
|
||
| ## Validation After Changes | ||
|
|
||
| After addressing all comments, run the full validation sequence: | ||
|
|
||
| 1. `gofmt -s -w .` then `git diff --exit-code` (formatting clean) | ||
| 2. `buf lint` (if any proto files changed) | ||
| 3. `ginkgo run -r internal` (unit tests pass) | ||
| 4. `go build ./cmd/fulfillment-service && go build ./cmd/osac` (both binaries compile) | ||
|
|
||
| ## Comment Responses | ||
|
|
||
| Write a JSON file to `.ai-bot/comment-responses.json` mapping each comment ID | ||
| to a short summary of what you did. The bot uses this to post descriptive | ||
| replies on the PR instead of generic messages. | ||
|
|
||
| ## Iteration Cap | ||
|
|
||
| Maximum 2 fix-test cycles per feedback round. If tests still fail after 2 | ||
| attempts, document the failure in the comment response and let the reviewer | ||
| decide. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| # Project Instructions | ||
|
|
||
| This is the fulfillment-service: a gRPC server with REST gateway for managing | ||
| infrastructure resources (clusters, hosts, compute instances, networking). It | ||
| uses PostgreSQL for storage, OPA for authorization, and supports Kubernetes | ||
| deployment via Helm/Kustomize. | ||
|
|
||
| Two binaries: `fulfillment-service` (server) and `osac` (CLI). | ||
|
|
||
| ## Validation Commands | ||
|
|
||
| Run these commands to validate your changes. All must pass before committing. | ||
|
|
||
| ### Build | ||
|
|
||
| ```bash | ||
| go build ./cmd/fulfillment-service | ||
| go build ./cmd/osac | ||
| ``` | ||
|
|
||
| ### Unit Tests | ||
|
|
||
| ```bash | ||
| # Run all unit tests (excludes integration tests in it/) | ||
| ginkgo run -r internal | ||
|
|
||
| # Run a specific package | ||
| ginkgo run internal/servers | ||
|
|
||
| # Run tests matching a pattern | ||
| ginkgo run -r internal --focus="CreateCluster" | ||
| ``` | ||
|
|
||
| Do NOT run `ginkgo run -r` (without restricting to `internal`). That triggers | ||
| integration tests which require a kind cluster and take a long time. | ||
|
|
||
| ### Code Formatting | ||
|
|
||
| ```bash | ||
| gofmt -s -w . | ||
| # Verify no files changed: | ||
| git diff --exit-code | ||
| ``` | ||
|
|
||
| ### Proto Linting and Code Generation | ||
|
|
||
| After editing any `.proto` file, always run both: | ||
|
|
||
| ```bash | ||
| buf lint | ||
| buf generate | ||
| ``` | ||
|
|
||
| Then verify no generated files changed unexpectedly: | ||
|
|
||
| ```bash | ||
| git diff --exit-code internal/api/ | ||
| ``` | ||
|
|
||
| ### Module Tidying | ||
|
|
||
| After editing `go.mod`: | ||
|
|
||
| ```bash | ||
| go mod tidy | ||
| ``` | ||
|
|
||
| ## Coding Standards | ||
|
|
||
| ### Architecture | ||
|
|
||
| - Public servers delegate to private servers and add tenant/auth logic. | ||
| `ClustersServer` (public) wraps `PrivateClustersServer` (private). | ||
| - Server files use builder pattern: `ClustersServerBuilder` configures dependencies. | ||
| - Both public and private server files live in `internal/servers/` | ||
| (e.g., `clusters_server.go` + `private_clusters_server.go`). | ||
| - Database layer uses `GenericDAO[O Object]` for type-safe CRUD on protobuf messages. | ||
| - Resources are stored as JSON-serialized protobuf in a `data` column. | ||
| - CEL filter expressions are translated to SQL WHERE clauses via `FilterTranslator`. | ||
|
|
||
| ### Testing | ||
|
|
||
| - Tests use **Ginkgo v2 + Gomega**. | ||
| - Suite setup in `*_suite_test.go` with `BeforeSuite` and `DeferCleanup`. | ||
| - `dao.CreateTables[T]()` dynamically creates test schemas. | ||
| - Mocks use `go.uber.org/mock` with `//go:generate mockgen` directives. | ||
| Mock files live alongside source (e.g., `attribution_logic_mock.go`). | ||
|
|
||
| ### Proto Structure | ||
|
|
||
| Protos are split into public and private APIs: | ||
|
|
||
| ```text | ||
| proto/public/osac/public/v1/ - User-facing API (read-heavy, limited write) | ||
| proto/private/osac/private/v1/ - Admin/controller API (full CRUD + Signal RPC) | ||
| proto/tests/osac/tests/v1/ - Test-only proto definitions | ||
| ``` | ||
|
|
||
| Each resource has `<resource>_type.proto` (messages) and `<resource>s_service.proto` (RPCs). | ||
| The `SERVICE_SUFFIX` buf lint rule is intentionally excluded in `buf.yaml`. | ||
|
|
||
| ## Files Never to Edit | ||
|
|
||
| These files and directories are generated or managed by external tools. | ||
| Never edit them manually: | ||
|
|
||
| - `internal/api/` -- fully generated by `buf generate` from proto files | ||
| - `go.sum` -- managed by `go mod tidy` | ||
| - `*_mock.go` files -- generated by `mockgen` via `//go:generate` directives | ||
| - `charts/` and `it/charts/` template directories -- Helm templates, not test code | ||
| - `dist/` -- build artifacts from goreleaser | ||
|
|
||
| ## Files to Be Cautious With | ||
|
|
||
| - `proto/**/*.proto` -- changes here cascade to generated code; always run | ||
| `buf lint && buf generate` after editing | ||
| - `internal/database/migrations/*.up.sql` -- existing migration files must | ||
| never be modified; only add new numbered migration files | ||
| - `.pre-commit-config.yaml`, `.goreleaser.yaml`, `buf.yaml`, `buf.gen.yaml` -- | ||
| infrastructure config; verify with maintainers before changing | ||
|
|
||
| ## Pre-commit Hooks | ||
|
|
||
| The repo uses pre-commit with these hooks: | ||
| - trailing-whitespace, check-merge-conflict, end-of-file-fixer | ||
| - check-added-large-files, check-case-conflict, check-json | ||
| - check-symlinks, detect-private-key | ||
| - yamllint (strict mode, excludes Helm chart templates) |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| # New Ticket Workflow | ||
|
|
||
| Read and execute `.ai-workflows/bugfix/skills/unattended.md` with these | ||
| settings: | ||
|
|
||
| - **branch**: Stay on the current branch (already created by the orchestration | ||
| system -- do not create a new branch). | ||
| - **lint_command**: `gofmt -s -w .` | ||
| - **iteration_cap**: Maximum 3 fix-test cycles before escalating. | ||
|
|
||
| All artifact paths (`.artifacts/bugfix/{issue}/`) should use `.ai-bot/` | ||
| instead. Write the PR description to `.ai-bot/pr.md`. | ||
|
|
||
| ## Repo-Specific Test Commands | ||
|
|
||
| Use these exact commands during the test phase: | ||
|
|
||
| ```bash | ||
| # Unit tests (mandatory -- always run) | ||
| ginkgo run -r internal | ||
|
|
||
| # Focused unit tests (use during iteration to speed up feedback) | ||
| ginkgo run -r internal --focus="<test pattern matching the fix area>" | ||
| ``` | ||
|
|
||
| Do NOT run integration tests (`ginkgo run it`). They require a kind cluster | ||
| with specific `/etc/hosts` entries and are validated separately by CI. | ||
|
|
||
| ## Repo-Specific Build Commands | ||
|
|
||
| ```bash | ||
| go build ./cmd/fulfillment-service | ||
| go build ./cmd/osac | ||
| ``` | ||
|
|
||
| ## After Proto Changes | ||
|
|
||
| If your fix touches any `.proto` file: | ||
|
|
||
| ```bash | ||
| buf lint | ||
| buf generate | ||
| ``` | ||
|
|
||
| Then verify the generated code compiles: | ||
|
|
||
| ```bash | ||
| go build ./cmd/fulfillment-service | ||
| ``` | ||
|
|
||
| ## After Mock Interface Changes | ||
|
|
||
| If your fix modifies an interface that has a `//go:generate mockgen` directive, | ||
| regenerate the mock: | ||
|
|
||
| ```bash | ||
| go generate ./path/to/package/ | ||
| ``` | ||
|
|
||
| ## Final Validation (Before Writing PR Description) | ||
|
|
||
| Run these in order. All must pass: | ||
|
|
||
| 1. `gofmt -s -w .` then `git diff --exit-code` (formatting) | ||
| 2. `buf lint` (proto linting, if protos changed) | ||
| 3. `ginkgo run -r internal` (full unit test suite) | ||
| 4. `go build ./cmd/fulfillment-service && go build ./cmd/osac` (both binaries) | ||
|
|
||
| ## Session Context | ||
|
|
||
| After completing the fix, write a session context summary to | ||
| `.ai-bot/session-context.md` covering: | ||
|
|
||
| - Root cause summary | ||
| - Files changed and why | ||
| - Test strategy (what was tested, what was not) | ||
| - Risks or areas that need human review |
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eranco74 is the bot supposed to add these files to the commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 0b9fbb5.