Skip to content

Q.a - #19

Merged
v-Kaefer merged 10 commits into
mainfrom
Q.A
Aug 10, 2026
Merged

Q.a#19
v-Kaefer merged 10 commits into
mainfrom
Q.A

Conversation

@v-Kaefer

@v-Kaefer v-Kaefer commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Linked Issue

  • Closes #

Milestone

Summary

How to test

  • Test type: automated | smoke | manual
  • Steps:

Evidence

  • Screenshot/GIF attached when applicable
  • Log/output attached when applicable
  • Manual checklist executed when applicable

Known risks

DoD checklist

  • Scope implemented as defined
  • Tests executed and documented
  • Evidence attached when applicable
  • No known critical breakage introduced

Summary by CodeRabbit

  • Workflow Improvements

    • Updated source-branch checks to run reliably for pull requests targeting the primary and QA branches.
    • Preserved existing branch validation requirements while improving workflow security.
  • Bug Fixes

    • Improved repository detection to distinguish the tool source repository from installed target projects.
    • Prevented target project files from incorrectly changing repository mode.
    • Validation now appropriately skips source-only checks in embedded projects.
  • Tests

    • Added coverage for installation, environment handling, Git setup, repository detection, and validation behavior.

@v-Kaefer v-Kaefer self-assigned this Aug 10, 2026
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The pull request changes source-branch workflows to use pull_request_target, adds an explicit repository marker, updates repository-mode detection and validation, and adds end-to-end and workflow contract tests.

Changes

Trusted source workflows

Layer / File(s) Summary
Source-branch workflow triggers
.github/workflows/main-source-branch.yml, .github/workflows/qa-source-branch.yml, tests/test_qa_workflows.py
Both source-branch workflows use pull_request_target. Contract tests verify the triggers and reject actions/checkout.

Repository mode validation

Layer / File(s) Summary
Explicit source marker detection
.project-setup-source, Makefile, scripts/validation/repo_quality.py
Repository detection uses .project-setup-source and requires the exact value github-project-setup-source. The marker is part of the source repository path contract.
Embedded-target validation
scripts/validation/repo_quality.py, tests/qa/test_repository_mode_e2e.py
Embedded targets skip caller-reference checks. End-to-end tests cover tool-like target files, preserved Makefiles, installation, environment cleanup, and Git setup.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Possibly related PRs

Sequence Diagram(s)

sequenceDiagram
  participant Installer
  participant TargetRepository
  participant Makefile
  participant RepoQuality
  Installer->>TargetRepository: install target files
  Makefile->>TargetRepository: check .project-setup-source
  TargetRepository-->>Makefile: marker absent or mismatched
  Makefile->>RepoQuality: run repository validation
  RepoQuality->>TargetRepository: validate required scripts
  RepoQuality-->>Installer: skip caller contracts for embedded target
Loading
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description reproduces the template but leaves the issue, milestone, summary, testing, evidence, risks, and DoD items incomplete. Complete each required section with the implementation details, test commands and results, evidence, risks, and completed DoD checklist items.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title "Q.a" is too vague and does not describe the workflow, repository validation, or test changes. Replace the title with a concise summary, such as "Harden source repository gates and add embedded repository tests."
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch Q.A

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tests/test_qa_workflows.py (1)

16-20: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Complete the trusted-source contract assertions.

The QA test checks branches: ["Q.A"] and HEAD_REF, but neither test checks the same-repository guard enforced at .github/workflows/qa-source-branch.yml Lines 35-38 and .github/workflows/main-source-branch.yml Lines 35-38. The main test also does not assert branches: ["main"]. Add these assertions. Otherwise, a fork pull request or a widened target filter can pass the tests while changing the source-gate contract. GitHub applies branches filters to the pull request target branch. (docs.github.com)

This follows the PR objective to validate trusted source workflows.

Suggested assertions
     def test_qa_source_requires_develop(self):
         ...
+        self.assertIn('HEAD_REPO" != "$EXPECTED_REPO', text)

     def test_main_source_requires_qa(self):
         ...
+        self.assertIn('branches: ["main"]', text)
+        self.assertIn('HEAD_REPO" != "$EXPECTED_REPO', text)

Also applies to: 22-25

🤖 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 `@tests/test_qa_workflows.py` around lines 16 - 20, Add assertions in the QA
workflow test for the same-repository guard used by qa-source-branch.yml and
assert the corresponding target branch filter. Update the main workflow test
similarly, including the same-repository guard and branches: ["main"], while
preserving the existing Q.A and HEAD_REF assertions.
🤖 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.

Nitpick comments:
In `@tests/test_qa_workflows.py`:
- Around line 16-20: Add assertions in the QA workflow test for the
same-repository guard used by qa-source-branch.yml and assert the corresponding
target branch filter. Update the main workflow test similarly, including the
same-repository guard and branches: ["main"], while preserving the existing Q.A
and HEAD_REF assertions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2e0ca10c-88c2-4d17-8d93-fd00428b0b48

📥 Commits

Reviewing files that changed from the base of the PR and between 5917620 and 1d2f12a.

📒 Files selected for processing (7)
  • .github/workflows/main-source-branch.yml
  • .github/workflows/qa-source-branch.yml
  • .project-setup-source
  • Makefile
  • scripts/validation/repo_quality.py
  • tests/qa/test_repository_mode_e2e.py
  • tests/test_qa_workflows.py

@v-Kaefer
v-Kaefer merged commit ee7ea25 into main Aug 10, 2026
16 of 18 checks passed
This was referenced Aug 10, 2026
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant