[jsweep] Clean validate_memory_files.cjs - #52227
Conversation
- Normalize custom allowed extensions so entries without a leading dot (e.g. 'json') are matched correctly against path.extname results - Add test coverage for the new normalization behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
✅ Great work! This PR looks ready for review. Summary: This PR fixes a latent bug in What's included:
The implementation is correct, the diff is minimal and focused, and the test suite verifies the fix works as expected. This is ready to merge!
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
❌ Test Quality Sentinel failed during test quality analysis. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100).
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
|
✅ PR Code Quality Reviewer completed the code quality review. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.individual.githubcopilot.com"See Network Configuration for more information.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The normalization fix is correct — path.extname() always returns dot-prefixed extensions (e.g. .json), so ensuring user-supplied extensions get a leading dot before Set lookup is the right fix. Test coverage is adequate.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 12.7 AIC · ⌖ 6.32 AIC · ⊞ 5.4K
There was a problem hiding this comment.
Pull request overview
Normalizes undotted custom memory-file extensions before validation.
Changes:
- Prefixes missing extension dots after trimming and lowercasing.
- Adds coverage confirming
"json"accepts.jsonfiles.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/validate_memory_files.cjs |
Normalizes extension allowlist entries. |
actions/setup/js/validate_memory_files.test.cjs |
Tests undotted extension handling. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
|
@copilot Please inspect the latest branch state, refresh the branch if needed, address the failed checks below, and run the No unresolved review threads were found.
Branch update was requested automatically for this run when GitHub allows it. Run context: https://github.com/github/gh-aw/actions/runs/31655027162
|
Summary
Cleaned up
actions/setup/js/validate_memory_files.cjsas part of the jsweep JS unbloater pass.Context type: Node.js utility invoked from a github-script step (uses the global
coreobject for logging; no@ts-nocheckwas present — file already used@ts-check).Changes
allowedExtensionsentries so values without a leading dot (e.g."json"instead of".json") are automatically prefixed with.before being compared againstpath.extname()results. Previously, an extension list like["json"]would silently reject all files becausepath.extname()always returns a leading-dot string.Test improvements
normalizes custom extensions missing a leading dot, verifying that a bare"json"entry inallowedExtensionscorrectly accepts.jsonfiles and rejects others.✅ Validation checklist
npm run format:cjs✓npm run lint:cjs✓npm run typecheck✓ (file already used@ts-check, no@ts-nocheckto remove)npm run test:js✓ (validate_memory_files.test.cjs: 34/34 passed)Run context: https://github.com/github/gh-aw/actions/runs/31655027162> Generated by 👨🍳 PR Sous Chef · gpt54 · 30.7 AIC · ⌖ 5.28 AIC · ⊞ 8.5K · ◷