OSAC-1739: Update osac-workspace and AI tooling for mono-repo - #179
Conversation
…ndalone clones Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tommy Hughes <tohughes@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tommy Hughes <tohughes@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tommy Hughes <tohughes@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tommy Hughes <tohughes@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tommy Hughes <tohughes@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tommy Hughes <tohughes@redhat.com>
Collapse the osac-project/fulfillment-service, osac-project/osac-operator, and osac-project/osac-aap entries in tools/pr-notify's repos list into a single osac-project/osac entry, matching the mono-repo merge. Also fixes the identical stale example block in tools/pr-notify/README.md. Verified generate.py/notify.py/github.py/data_formatter.py/formatter.py have no repo-name-keyed or repo-count assumptions -- all grouping is dynamic, so no code changes are needed alongside the config update. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tommy Hughes <tohughes@redhat.com>
Fix design-review/SKILL.md's fulfillment-service/docs/API.md path to the new osac/-prefixed location. Update github-actions-workflows self-check.sh's SELF_CHECK_REPO default from the standalone osac-operator repo to the osac mono-repo, and fix a latent bug this surfaced: gh api's raw JSON error body on a 404 lands on stdout (not stderr), so `gh api ... 2>/dev/null || true` + an output-emptiness check misreads a failed lookup as success. This was latent only because the old default repo still had its v0.0.1 tag; the new default (osac-project/osac) has no tags yet post-merge, which turned the bug into a false FAIL instead of a graceful skip. Fixed by checking the actual exit status of both gh api calls. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tommy Hughes <tohughes@redhat.com>
Fix a broken relative link (fulfillment-service/docs/API.md) in .design/templates/section-guidance.md - a call site the per-task reviews missed since it's outside the plan's 8 named files. Also update two stale-but-not-broken prose references in .design/context/osac-dimensions.md and skills/design-review/SKILL.md's calibration examples for consistency with the rest of this migration. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tommy Hughes <tohughes@redhat.com>
Signed-off-by: Tommy Hughes <tohughes@redhat.com>
|
@tchughesiv: This pull request references OSAC-1739 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tchughesiv The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThe workspace now treats ChangesOSAC monorepo migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 9 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/rules/cross-repo-workflow.md:
- Around line 25-30: Update the git worktree setup commands in the documented
workflow to create feature-branch from HEAD when adding the worktree by using
the branch-creation option with git worktree add; leave the subsequent directory
change and cleanup commands unchanged.
In @.gitignore:
- Line 60: Retain the root-scoped ignore rules for fulfillment-service/,
osac-operator/, and osac-aap/ in .gitignore at lines 60-60 until legacy
quarantine completes. Also retain the matching Skillsaw exclusions in
.skillsaw.yaml at lines 388-391 during the same migration window; update both
sites and do not remove these protections.
In `@kind-dev/setup.sh`:
- Line 1072: Update the existing osac-aap project fallback in setup.sh to patch
its scm_url to https://github.com/osac-project/osac.git, trigger a project sync,
and validate every API response. Reconcile same-named job templates to use the
monorepo playbook paths, or delete and recreate them when updating is not
supported, ensuring failures stop execution rather than leaving stale AWX
resources.
In `@skills/create-pr/SKILL.md`:
- Line 3: Update the validation steps in SKILL.md for the fulfillment-service
and osac-aap component blocks to match each component’s AGENTS.md matrix: add
`uv run dev.py lint` for fulfillment-service, add `make test` for osac-aap, and
invoke ansible-lint through `uv run ansible-lint` instead of directly. Ensure
every affected component runs its complete required validation before the PR
workflow proceeds.
- Around line 134-144: Update the file-matching rules in the classification step
to explicitly cover osac-aap production plugins under osac-aap/plugins/**/*.py
and root-level AAP tests under osac-aap/test_*.py, preserving the osac-aap/
prefix in every pattern. Ensure Step 3 uses these mappings so changed Python
files are classified without false unmatched-file warnings.
- Around line 49-53: Update the TOUCHED_COMPONENTS pipeline to preserve failures
from git diff while still treating no matching component directories as an empty
result. Remove the trailing || true and use a filtering approach that returns
success when there are no matches, without masking upstream failures.
In `@skills/github-actions-workflows/scripts/self-check.sh`:
- Around line 139-142: Update the tag-reference lookups in the self-check flow
around ref_json and GUARDED_SHA so gh api diagnostics are preserved instead of
redirected to /dev/null. Capture each command’s combined output and include the
diagnostic text in the corresponding skip or failure message, while retaining
the existing behavior for successful lookups and expected non-tag cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9b9215dd-c26c-4066-8343-9513c1b7aeb8
📒 Files selected for processing (18)
.claude/rules/cli-ux.md.claude/rules/cross-repo-workflow.md.claude/rules/protobuf-conventions.md.design/context/osac-dimensions.md.design/templates/section-guidance.md.gitignore.skillsaw.yamlAGENTS.mdREADME.mdbootstrap.shkind-dev/README.mdkind-dev/setup.shskills/create-pr/SKILL.mdskills/design-review/SKILL.mdskills/github-actions-workflows/reference.mdskills/github-actions-workflows/scripts/self-check.shtools/pr-notify/README.mdtools/pr-notify/config.example.toml
- Fix git worktree example missing -b (would error on a genuinely new branch, the doc's own stated use case) - Restore fulfillment-service/, osac-operator/, osac-aap/ entries in .gitignore and .skillsaw.yaml until legacy quarantine has soaked, protecting pre-bootstrap workspaces from a git-staging/lint-scope gap - Patch and resync a reused AWX osac-aap project's scm_url in kind-dev/setup.sh instead of leaving a stale pre-mono-repo URL - create-pr skill: fill validation-matrix gaps (fulfillment-service missing uv run dev.py lint; osac-aap missing make test, calling ansible-lint directly instead of via uv) - create-pr skill: stop `|| true` from masking a genuine git diff failure in TOUCHED_COMPONENTS detection; use awk instead, which exits 0 on zero matches without needing a blanket failure-swallow - create-pr skill: add missing Python matching rule for osac-aap plugins in the test-coverage classification table - self-check.sh: stop discarding gh api diagnostics on the tag/SHA resolution path so the skip message shows why, matching the pattern already used a few lines below in the same script Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tommy Hughes <tohughes@redhat.com>
- Cross-reference OSAC-1737 (repo archival ticket) in the .gitignore/ .skillsaw.yaml migration-window comments, so the temporary dual-listed entries have a concrete trigger for removal instead of risking being forgotten indefinitely - create-pr skill: don't silently skip validation for root-level osac changes (go.work, root Makefile, .github/workflows/) that touch none of the 3 component subdirectories but can still break multiple components' builds (e.g. a broken go.work affects both fulfillment-service and osac-operator) Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tommy Hughes <tohughes@redhat.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/create-pr/SKILL.md (1)
214-217: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the missing AI commit trailer.
0510ac2 OSAC-1739: address cross-cutting review findingshasSigned-off-by:but no Red Hat-accepting AI trailer. AddGenerated-by:orAssisted-by:with the Red Hat identity for this commit. NoCo-Authored-By:AI trailer was found.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/create-pr/SKILL.md` around lines 214 - 217, Update the commit guidance for the shared osac mono-repo PR flow around the cross-cutting review commit to require a Red Hat-accepted AI trailer, using either Generated-by: or Assisted-by: with the appropriate Red Hat identity alongside Signed-off-by:.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@skills/create-pr/SKILL.md`:
- Around line 214-217: Update the commit guidance for the shared osac mono-repo
PR flow around the cross-cutting review commit to require a Red Hat-accepted AI
trailer, using either Generated-by: or Assisted-by: with the appropriate Red Hat
identity alongside Signed-off-by:.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ce28ea20-3c8f-4a65-804a-66c680973f6a
📒 Files selected for processing (6)
.claude/rules/cross-repo-workflow.md.gitignore.skillsaw.yamlkind-dev/setup.shskills/create-pr/SKILL.mdskills/github-actions-workflows/scripts/self-check.sh
OSAC-1739: Update osac-workspace and AI tooling for mono-repo
Jira: https://redhat.atlassian.net/browse/OSAC-1739
Summary
fulfillment-service,osac-operator, andosac-aaphave been merged intothe new
osac-project/osacmono-repo (per OSAC-1732'sper-component cutover plan). This PR updates
osac-workspace—bootstrap.sh,root docs,
.claude/rules/,.skillsaw.yaml,kind-dev/, thecreate-prandperipheral skills, and
tools/pr-notify/— so cloning, routing, cross-componentworkflow guidance, and PR automation correctly reflect the merged layout
instead of the old 3-separate-repo one.
osac-installerandbare-metal-fulfillment-operatorare untouched — both remain standalone,not yet merged (update incrementally as each cuts over, per the epic).
Changes
bootstrap.sh/.gitignore— cloneosac-project/osaconce intoosac/instead of 3 separate repos; auto-quarantine (never delete) anypre-existing standalone clones of the 3 merged components to
.legacy-repos/<name>/, preserving all local git state.AGENTS.md,README.md) — component table, build/testcommands, cross-component-change guidance, and architecture diagram
updated for the mono-repo subdirectory layout. Also fixed a stale
OpenShift deployment command referencing a
manifests/directory removedwhen the project switched to Helm.
.claude/rules/*.md— routing tables and path references incross-repo-workflow.md,protobuf-conventions.md, andcli-ux.mdupdated to the
osac/-prefixed paths..skillsaw.yaml— collapsed 3 per-component lint excludes into oneosac/**entry; added.legacy-repos/**.kind-dev/— path references updated; AAPscm_urlnow points atosac-project/osacwith playbook paths prefixedosac-aap/(AWX has nonative subdirectory-scoped checkout).
skills/create-pr/SKILL.md— Step 1 now detects which merged-componentsubdirectories a diff touches (a single PR can span more than one) instead
of assuming "current repo = current component."
tools/pr-notify/— collapsed 3 monitored repos intoosac-project/osac.design-review/SKILL.mdandgithub-actions-workflows/{reference.md,scripts/self-check.sh}path/defaultupdates. Fixed a latent bug in
self-check.shsurfaced by the default-repochange (a failed
gh apicall's error body was being misread as success).Testing
tools/pr-notify's existing suite (59 tests) re-verifiedpassing; no test code needed changes (repo-list handling is fully generic).
application code (shell/Markdown/YAML tooling only).
make skillsaw(0 errors/warnings),shellcheckon every modified script(no new issues vs.
main), and a live end-to-end run —bootstrap.shwas actually executed against this real machine's pre-existing standalone
clones (confirmed clean: no uncommitted/unpushed work) and against a fresh
environment, successfully cloning
osac/and quarantining the old clonesto
.legacy-repos/with git history fully intact and idempotently. Everyliteral path referenced in the updated docs/skills was verified to resolve
against the real local
osac/clone. Thecreate-prskill's newmulti-component detection logic was dry-run against a real multi-component
commit inside the cloned
osacrepo and correctly identified both touchedcomponents.
.artifacts/implement/OSAC-1739/05-validation-report.md.Acceptance Criteria
bootstrap.shclones the mergedosacrepo instead of 3 separate reposAGENTS.md,README.md,CLAUDE.md) reflect new layout.claude/rules/*.mdcomponent routing and paths updated.skillsaw.yamlexcludes match new layoutkind-dev/paths and AAPscm_urlresolve correctlycreate-prskill correctly detects/validates mono-repo PRspr-notifymonitors the correct repo(s)skills/osac-release/,.ai-bot/,osac-installer/,bare-metal-fulfillment-operator/, image-tag refs)Summary by CodeRabbit
New Features
Bug Fixes
Documentation