Skip to content

chore(ci): wire required PR/push validation workflow (#1278) #1284

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.

Sign up for GitHub

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

Merged
ggfevans merged 4 commits into from
Feb 25, 2026
Merged

chore(ci): wire required PR/push validation workflow (#1278) #1284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
89014b4
chore(ci): wire required PR/push validation workflow (#1278)
ggfevans Feb 19, 2026
6f7b4eb
fix(lint): resolve 7 pre-existing lint errors exposed by CI validation
ggfevans Feb 25, 2026
59e8fd5
Merge branch 'main' into chore/1278-wire-required-ci-gates
ggfevans Feb 25, 2026
1e33342
Merge branch 'main' into chore/1278-wire-required-ci-gates
ggfevans Feb 25, 2026
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
11 changes: 10 additions & 1 deletion .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
name: Test

on:
pull_request:
branches: [main]
push:
branches: [main]
workflow_dispatch:
workflow_call:

concurrency:
group: test-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref || github.run_id }}
cancel-in-progress: true
Comment on lines +11 to +13
Copy link

@coderabbitai coderabbitai bot Feb 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat .github/workflows/test.yml

Repository: RackulaLives/Rackula

Length of output: 1465

🏁 Script executed:

# Find workflows that call test.yml as a reusable workflow
rg -rn --glob "*.yml" --glob "*.yaml" "workflows/test" .github/workflows/ | head -30

Repository: RackulaLives/Rackula

Length of output: 124

🏁 Script executed:

# Check git history for test.yml to see if concurrency was recently added
git log --oneline -20 -- .github/workflows/test.yml

Repository: RackulaLives/Rackula

Length of output: 132

🏁 Script executed:

# Search for uses: directives in workflows
rg -A2 "uses:" .github/workflows/ --glob "*.yml" --glob "*.yaml" | grep -i "test" | head -40

Repository: RackulaLives/Rackula

Length of output: 722

🏁 Script executed:

# Search for any references to test.yml in workflow files using uses:
rg "uses:.*test\.yml" .github/workflows/ --glob "*.yml" --glob "*.yaml"

Repository: RackulaLives/Rackula

Length of output: 46

🏁 Script executed:

# Get more context on the git commit that added this
git show 89014b4 --stat

Repository: RackulaLives/Rackula

Length of output: 50377

🏁 Script executed:

# Search for any uses directives that reference test
rg "uses:" .github/workflows/ --glob "*.yml" --glob "*.yaml" -A 1 | grep -i "test" -A 1 -B 1

Repository: RackulaLives/Rackula

Length of output: 510

🏁 Script executed:

# Search for all uses: directives in workflow files to see the pattern
rg "uses:" .github/workflows/ --glob "*.yml" --glob "*.yaml" | grep -v "actions/" | head -20

Repository: RackulaLives/Rackula

Length of output: 2438

🏁 Script executed:

# Check if test.yml actually was previously a file (to confirm regression claim)
git log --all --full-history -- ".github/workflows/test.yml" | head -20

Repository: RackulaLives/Rackula

Length of output: 756

🏁 Script executed:

# Check all workflow files for any reference to test workflow
find .github/workflows -name "*.yml" -o -name "*.yaml" | xargs grep -l "test" | head -10

Repository: RackulaLives/Rackula

Length of output: 412

🏁 Script executed:

# Verify the actual date test.yml was added
git log --follow --format="%H %ai" -- .github/workflows/test.yml | tail -5

Repository: RackulaLives/Rackula

Length of output: 130

🏁 Script executed:

# Check if the review is accurate about regression by looking at git diff
git show 89014b4:.github/workflows/test.yml 2>/dev/null | head -20

Repository: RackulaLives/Rackula

Length of output: 435

Correct the concurrency expression to safely guard workflow_call invocations.

The concurrency group expression has a logic issue: for workflow_call events, github.event.pull_request.number is never populated (the event type is workflow_call, not pull_request), and github.ref reflects the caller's ref. This means concurrent invocations from a single caller could unnecessarily cancel each other. Additionally, github.ref is always non-empty, making the || github.run_id fallback unreachable.

While test.yml currently has no caller workflows in the repository, the workflow_call trigger is declared in the on: section. To future-proof and correctly handle all event types:

🔧 Proposed fix 
 concurrency:
-  group: test-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref || github.run_id }}
+  group: test-${{ github.workflow }}-${{ github.event.pull_request.number || (github.event_name == 'workflow_call' && github.run_id) || github.ref }}
   cancel-in-progress: true

With this expression:

  • pull_request → uses PR number (deduplicates reruns on the same PR) ✅
  • push to main → uses ref (cancels superseded pushes) ✅
  • workflow_dispatch → uses ref (cancels duplicate dispatches on same branch) ✅
  • workflow_call → uses unique run_id (each invocation is independent, no mutual cancellation) ✅
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
concurrency:
group: test-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref || github.run_id }}
cancel-in-progress: true
concurrency:
group: test-${{ github.workflow }}-${{ github.event.pull_request.number || (github.event_name == 'workflow_call' && github.run_id) || github.ref }}
cancel-in-progress: true
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/test.yml around lines 11 - 13, The concurrency group
expression currently uses github.event.pull_request.number || github.ref ||
github.run_id which misbehaves for workflow_call and makes github.run_id
unreachable; update the concurrency group (the line that builds group: test-${{
github.workflow }}-...) to pick the identifier based on github.event_name: use
github.event.pull_request.number when github.event_name == 'pull_request', use
github.run_id when github.event_name == 'workflow_call', and otherwise use
github.ref (for push/workflow_dispatch), so each event type uses the correct
unique key and workflow_call invocations do not cancel each other.


jobs:
test:
validate:
name: validate
runs-on: ubuntu-latest
permissions:
contents: read
Expand Down
4 changes: 2 additions & 2 deletions api/src/storage/filesystem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ async function migrateLegacyLayout(
try {
parsed = yaml.load(yamlContent, { schema: yaml.JSON_SCHEMA });
} catch (e) {
throw new Error(`Invalid YAML: ${e instanceof Error ? e.message : e}`);
throw new Error(`Invalid YAML: ${e instanceof Error ? e.message : e}`, { cause: e });
Copy link

@coderabbitai coderabbitai bot Feb 25, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Align non-Error fallback with the explicit String(e) pattern used in saveLayout.

Line 338 uses implicit template-literal coercion for the non-Error branch (${... : e}), while line 470 uses the explicit String(e). They're functionally equivalent for all values, but the inconsistency is unnecessary.

♻️ Proposed alignment 
-    throw new Error(`Invalid YAML: ${e instanceof Error ? e.message : e}`, { cause: e });
+    const message = e instanceof Error ? e.message : String(e);
+    throw new Error(`Invalid YAML: ${message}`, { cause: e });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
throw new Error(`Invalid YAML: ${e instanceof Error ? e.message : e}`, { cause: e });
const message = e instanceof Error ? e.message : String(e);
throw new Error(`Invalid YAML: ${message}`, { cause: e });
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@api/src/storage/filesystem.ts` at line 338, Replace the inconsistent
non-Error fallback in the throw statement so it matches the explicit String(e)
pattern used in saveLayout: update the throw in filesystem.ts (the "throw new
Error(`Invalid YAML: ${e instanceof Error ? e.message : e}`..." occurrence) to
use String(e) for the non-Error branch (i.e., keep e instanceof Error ?
e.message : String(e)) so the thrown Error message construction is consistent
with saveLayout's error handling.

}

const layout = LayoutFileSchema.safeParse(parsed);
Expand Down Expand Up @@ -467,7 +467,7 @@ export async function saveLayout(
parsed = yaml.load(yamlContent, { schema: yaml.JSON_SCHEMA });
} catch (e) {
const message = e instanceof Error ? e.message : String(e);
throw new Error(`Invalid YAML: ${message}`);
throw new Error(`Invalid YAML: ${message}`, { cause: e });
}

// Validate layout schema
Expand Down
11 changes: 11 additions & 0 deletions docs/guides/TESTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,17 @@ Use it to:

**Note:** Dev deployment only succeeds if lint and tests pass.

### Required CI Checks

Branch protection should require the core CI validation check from `.github/workflows/test.yml`:

- **Check name:** `Test / validate`

This check runs on:

- Pull requests targeting `main` (pre-merge gate)
- Pushes to `main` (post-merge validation)
Comment on lines +107 to +116
Copy link

@coderabbitai coderabbitai bot Feb 19, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

### Required CI Checks is a semantic mismatch inside ## Environments.

CI branch-protection check configuration is not an environment topic. As the heading hierarchy stands, this section appears as a child of ## Environments, which is about deployment targets (Local / Dev / Prod). Readers looking for branch-protection setup are unlikely to search there.

Consider either promoting it to a top-level ## section (e.g., between ## Environments and ## Philosophy) or grouping it under the adjacent "Dev Environment" flow as a note rather than a sibling heading.

As per coding guidelines: docs/**/*.md — "Organize documentation by purpose: ARCHITECTURE.md for high-level overview, guides/ for testing and accessibility patterns, reference/ for specifications and technical details, planning/ for roadmap."

🧰 Tools
🪛 LanguageTool

[uncategorized] ~109-~109: The official name of this software platform is spelled with a capital “H”.
Context: ...quire the core CI validation check from .github/workflows/test.yml: - Check name:...

(GITHUB)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@docs/guides/TESTING.md` around lines 107 - 116, The "### Required CI Checks"
heading is incorrectly nested under "## Environments"; move the "### Required CI
Checks" section out of the Environments block and promote it to a top-level "##
Required CI Checks" (or place it between "## Environments" and "## Philosophy")
so it stands alongside other top-level guide sections, or alternatively convert
it into a note inside the "Dev Environment" subsection; update the heading level
from "###" to "##" and adjust surrounding whitespace so the section appears at
the document root rather than as a child of "## Environments".


## Philosophy

We follow the **Testing Trophy** approach:
Expand Down
3 changes: 2 additions & 1 deletion scripts/generate-netbox-homelab-candidates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ function getNetBoxGitInfo(netboxRoot: string): {
const message = error instanceof Error ? error.message : String(error);
throw new Error(
`Failed to read git metadata from NetBox root "${netboxRoot}": ${message}`,
{ cause: error },
);
}
}
Expand All @@ -472,7 +473,7 @@ function collectCandidates(
if (!file.endsWith(".yaml")) continue;

const yamlPath = join(vendorDir, file);
let parsed: NetBoxDoc | null = null;
let parsed: NetBoxDoc | null;
try {
parsed = yaml.load(readFileSync(yamlPath, "utf8")) as NetBoxDoc;
} catch (error) {
Expand Down
4 changes: 2 additions & 2 deletions scripts/import-netbox-devices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@ async function importDevice(
// e.g., device.slug = 'hpe-proliant-dl360-gen10'
const imageSlug = device.slug;

let frontImage = false;
let rearImage = false;
let frontImage: boolean;
let rearImage: boolean;

// Try to download front image (try .png first, then .jpg)
const frontDest = join(destDir, `${imageSlug}.front.png`);
Expand Down
Loading