Skip to content

Add CI check to prevent unescaped braces in directory names#183

Open
joryirving wants to merge 1 commit into
mainfrom
foreman/wl-misospace-kubetix-123/issue-123
Open

Add CI check to prevent unescaped braces in directory names#183
joryirving wants to merge 1 commit into
mainfrom
foreman/wl-misospace-kubetix-123/issue-123

Conversation

@joryirving

Copy link
Copy Markdown
Collaborator

Fixes #123

Coded by Saffron (foreman coder, llama-nvidia) — Workload wl-misospace-kubetix-123: gate GATE-PASS, Ornith review GO (24 turns).

Opened manually: foreman currently ends at 'reviewed branch pushed' — PR creation is the missing last hop (upstream issue incoming).

The brace-named directory kubetix-api/kubetix_api/{models,schemas,database,auth,oidc,grants,teams}/
was already cleaned up. Added a new 'directory-check' job to the CI workflow that fails
if any directory name contains literal { or } characters, which are typically left over
from failed brace expansion.

Fixes #123

Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com>

@its-saffron its-saffron 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.

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)

PR Review: PR 183 - Add CI check to prevent unescaped braces in directory names

Recommendation: Request Changes

This PR addresses only half of linked issue PR 123's requirements. While the CI check implementation is sound, the issue explicitly requires two actions:

  1. Delete the empty kubetix-api/kubetix_api/{models,schemas,database,auth,oidc,grants,teams}/ directory
  2. Add a CI check that fails if any directory name contains unescaped braces

This PR implements only item 2. The deletion of the problematic empty directory is missing from the diff.


Change-by-Change Findings

.github/workflows/ci.ymldirectory-check job (lines 124–140)

Aspect Finding
Purpose Fails CI if any directory contains literal { or } characters
Detection method find . -type d -name '*{*' -o -type d -name '*}*'
Error output Echoes the found paths before exiting with code 1
Action pinned actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 (v7.0.0), consistent with other jobs
Trigger Runs on push to main and on PRs (inherits on: [push, pull_request] from workflow root)
CI result directory-check completed with success

Positive observations:

  • The check uses -type d to search only directories, avoiding false positives in filenames.
  • Both { and } are checked independently, catching single-brace directory names.
  • The exit 1 is reached only when found is non-empty, avoiding spurious failures.
  • The step name and comment clearly describe intent.

Potential edge case:

  • The pattern -name '*{*' matches directories containing a { anywhere; -name '*}*' matches directories containing a } anywhere. This correctly catches the literal-brace directory {models,schemas,...}.
  • However, this will also catch valid directory names that legitimately contain braces (e.g., prefix-{branch}-suffix). The corpus shows no such directories exist in this repository, but if they are added later, this check would need refinement (e.g., whitelist patterns or more precise glob matching).

Standards Compliance

Standard Status
Repository conventions (AGENTS.md) ✅ Follows existing CI job structure and action pinning
Workflow file patterns ✅ Consistent with actions/checkout v7.0.0 used elsewhere
Bash safety Minor: found=$(find ...) — unquoted is safe here because the variable is only tested with [ -n ] and echoed (not re-parsed), but quoting $"$found" in the echo would be more defensive

Linked Issue Fit

Requirement from PR 123 Addressed by this PR?
Delete the empty brace-named directory Missing — the diff contains no file deletion
Add CI check for unescaped braces ✅ Implemented

Issue PR 123 explicitly states: "Delete the empty kubetix-api/kubetix_api/{models,schemas,database,auth,oidc,grants,teams}/ directory and add a CI check..." Both parts must be addressed. The CI check passing suggests the directory was deleted prior to this PR, but the deletion is not in the diff and not mentioned in the PR body.


Evidence Provider Findings

No evidence providers were configured for this review.


Tool Harness Findings

All requested tools completed successfully:

  • read_file on .github/workflows/ci.yml — confirmed full workflow content
  • git_log — confirmed single-commit PR (8734320)
  • git_status_short — no untracked changes in diff
  • git_diff_stat — no staged changes (clean working tree)

Unknowns / Needs Verification

  1. Empty directory deletion: The diff does not show deletion of kubetix-api/kubetix_api/{models,schemas,database,auth,oidc,grants,teams}/. Verify whether:

    • The directory was deleted in a prior commit on this branch (not visible in diff)
    • The directory was never present on the branch
    • The directory still exists but the CI check will catch it on the next run

    Recommended action: Confirm the directory is gone and document how it was removed, or add the deletion to this PR to make it self-contained.

  2. Directory check robustness: If legitimate brace-containing directory names are introduced in the future, this check may produce false positives. Consider adding a comment or documentation note about this constraint.


Summary

The CI check implementation is correct and passes all CI gates. However, the PR is incomplete per the linked issue's acceptance criteria — the empty brace-named directory deletion is not present in the diff. To fully close issue PR 123, either:

  • Confirm the directory was already removed (provide evidence) and acknowledge the PR as complete for the CI check, or
  • Add the directory deletion to this PR so it is self-contained

Blocking finding: The directory deletion from issue PR 123 is not present in the diff.

Verdict derived from structured findings (verdict_policy=findings_severity_gated): 0 blocker finding(s) out of 1; model verdict was 'request_changes'.

Comment thread .github/workflows/ci.yml
- name: Template Helm chart
run: |
helm template kubetix-test ./charts/kubetix > /dev/null

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Major (docs): Linked issue PR 123 requires deleting the empty 'kubetix-api/kubetix_api/{models,schemas,database,auth,oidc,grants,teams}/' directory AND adding a CI check, but this PR only contains the CI check. The deletion is absent from the diff and the PR body does not document how the directory was removed.

Automated finding from AI PR review.

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.

Delete empty brace-named directory and add CI check to prevent unescaped braces

2 participants