Skip to content

fix(ci): restore retained Mermaid permissions - #166

Open
0monish wants to merge 1 commit into
mainfrom
agent/kel-152-restore-mermaid-permissions
Open

fix(ci): restore retained Mermaid permissions#166
0monish wants to merge 1 commit into
mainfrom
agent/kel-152-restore-mermaid-permissions

Conversation

@0monish

@0monish 0monish commented Sep 4, 2026

Copy link
Copy Markdown
Member

Summary

  • restore a failed or explicitly retained MSYS Mermaid output directory from temporary 0777 to owner-only 0700
  • validate the random render-path prefix before chmod and preserve the renderer's original nonzero status through EXIT cleanup
  • strengthen CI hygiene with deletion, broad-mode, platform-gating, path-bounds, ordering, and failure-status mutations
  • park this PR until KEL-152's real Windows Docker failure/ACL oracle is recorded

Spec refs

KEL-152. No boundary change: this changes only permissions on one gate-owned random temporary directory during cleanup.

Review gates

unsafe: none; public API: none; permission model: none (product permission model); dependency addition: none; wire protocol: none. CI shared-file review completed at 73f66d4: CodeRabbit exact committed diff, 0 findings.

Tests

  • failure-first: checker unit suite passed, then checkout validation failed because the unfixed renderer lacked restore_docker_output_dir()
  • just ci: passed; 630/630 workspace tests, 2 intentional skips
  • just hygiene: 83/83 plus checkout validation
  • just mermaid-render-check: 9/9 accessible SVGs using Mermaid CLI 11.16.0 image sha256:29077c6bd02f14bdfdd5fee552d9c00fe68d4fab3cd84952d21e2d1faf2fadaf
  • bash -n tools/mermaid_render_check.sh; git diff --check

Platforms

macOS arm64: full local gate and real Docker success/cleanup path passed. Windows: required real Git Bash + Docker Desktop forced-render-failure permission/ACL observation is unavailable on this device and remains merge-blocking. No Windows pass is claimed. Linux: hosted routed checks pending.

Perf impact

None expected; cleanup adds one MSYS-only chmod only when output is retained.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 17 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: ed307988-95f6-47e1-ae49-20d3b37d424f

📥 Commits

Reviewing files that changed from the base of the PR and between dcc4676 and 73f66d4.

📒 Files selected for processing (2)
  • tools/ci_hygiene.rs
  • tools/mermaid_render_check.sh

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.

@0monish

0monish commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

Additional failure-path evidence at exact tip 73f66d4:

  • Ran the real pinned renderer with PATH shims that report MSYS_NT, preserve host paths, provide GNU-style chmod MODE -- PATH, and force only docker run to exit 42 after Docker info/context/image checks.
  • The renderer preserved its own nonzero result (status=1), printed the exact retained /tmp/keld-mermaid-render.* path, kept that directory inspectable, and restored mode 700.
  • All temporary shim/output directories were removed after inspection.

This proves cleanup ordering and exit-status preservation through the real script/Docker preflight on macOS under simulated MSYS dispatch. It does not prove Git Bash/NTFS ACL mapping, so the real Windows Docker acceptance remains open and the PR remains do-not-merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M 50-199 lines type:fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants