fix: validate app directory slugs - #130
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Approve posted as a comment — Perry's review found this PR clean and would approve it, but the maintainer app that carries Perry's approve identity isn't installed on
OpenRouterTeam, so this posts as a comment instead. A human reviewer can approve.
Perry's Review
This PR renames the one app directory that violated the documented slug convention (apps/Maxim AI → apps/maxim-ai) and adds a validator check (APP_DIR_PATTERN = /^[a-z0-9-]+$/) in scripts/validate.js that enforces lowercase-letters/numbers/hyphens on every app directory name — matching the CONTRIBUTING.md guidance. All existing app dirs already satisfy the pattern, so the new check breaks nothing. The validate CI check is green.
The validator logic is sound: the pattern check runs before the directory-existence check, reports a clear actionable message, and the regex matches the documented convention exactly. On the blast radius: the README and apps.json are both derived from the directory name by the repo's generate script, and CONTRIBUTING.md says not to edit the README directly because a GitHub Action regenerates it on merge to main. So the stale image path and slug that the rename leaves behind self-heal on merge — no edit needed from the author, and this correctly follows the contributing workflow.
Verdict: ✅ LGTM — clean validation addition + correct rename; the derived-file staleness is by-design and self-heals on merge.
Risk: 🟢 Low
Risk assessment:
| Dimension | Severity | Risk | Reasoning |
|---|---|---|---|
| Implementation risk | 🟩 | Low | 8 lines of straightforward regex validation; pattern matches the documented convention; no control-flow or error-handling edge cases. |
| Premise risk | 🟩 | Low | The diagnosis (one dir violates the slug rule) and the approach (validate + rename the offender) are both correct and confirmed against the repo. |
| Estimated impact | 🟩 | Low | Worst case the validator rejects a future contributor's dir name — which is the intended guardrail, not a regression. No user-facing or data path is touched. |
Risk factors
| Risk Factor | Severity | Risk | Reasoning |
|---|---|---|---|
| Reversibility | 🟩 | Low | Pure additive validation; revertable by removing the check. |
| Detectability | 🟩 | Low | CI validate check surfaces any violation immediately. |
| Blast radius | 🟩 | Low | Single validation script; no other consumers of the new pattern. |
| Data integrity | None | None | No persisted state touched. |
| Financial exposure | None | None | No billing or payment path. |
| Security and privacy exposure | None | None | No auth, secrets, or tenant data involved. |
| Propagation | 🟩 | Low | Derived README and apps.json regenerate on merge; stale refs are transient. |
| Availability | None | None | No serving path affected. |
| Recovery cost | None | None | A bad rename is a one-line git revert. |
| Time to correct | 🟩 | Low | Any issue is caught at CI time, same PR. |
Renames
apps/Maxim AI→apps/maxim-aiand adds directory-name validation toscripts/validate.js(must be lowercase letters, numbers, and hyphens only).Landing this first is safe on its own — the current openrouter-web sync copes fine with a renamed slug. Should land before/independent of #38342 (Devin Review); only the reverse order is risky (if that PR merges while upstream still says "Maxim AI", the next 6am UTC daily sync fails slug validation until this rename lands).
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>