[jsweep] Clean validate_memory_files.cjs#15313
Conversation
… and optional chaining
|
@copilot lint, fix tests |
|
@copilot merge main and fix lint js |
…15319) * Add refiner workflow for automated code style and security analysis (#15307) * test: avoid flaky readonly dir assertion in extractZipFile (#15312) * docs: add unassign-from-user safe output documentation (#15310) * Fix TypeScript type errors in error handling for unknown types (#15308) * Initial plan --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Mark L <73659136+liuxiaopai-ai@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes the GitHub-script-side memory validation utility while also introducing/refining several workflow, docs, and test behaviors across the repo.
Changes:
- Modernize
actions/setup/js/validate_memory_files.cjs(typedefs, optional chaining, simplified error formatting). - Improve robustness/consistency in error handling and tests (JS
instanceof Errorguards; Go test avoids flakiness under elevated permissions). - Add/adjust workflow and documentation pieces (new
refinerworkflow + lock, safe-outputs docs addition, smoke schedule cron adjustment).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/validate_memory_files.cjs |
Refactors memory file extension validation logic for readability/typing. |
actions/setup/js/safe_output_handler_manager.cjs |
Hardens error-to-string conversion in PR review submission path. |
actions/setup/js/safe_output_unified_handler_manager.cjs |
Same error handling hardening as the non-unified manager. |
actions/setup/js/merge_remote_agent_github_folder.cjs |
String-quote normalization and whitespace cleanup. |
pkg/cli/logs_extract_zip_test.go |
Makes extraction error test non-flaky when running with elevated permissions. |
docs/src/content/docs/reference/safe-outputs.md |
Documents unassign-from-user safe output type with example config. |
.github/workflows/refiner.md |
Adds a new “Code Refiner” agentic workflow manifest. |
.github/workflows/refiner.lock.yml |
Adds compiled lock workflow for refiner.md. |
.github/workflows/smoke-copilot.lock.yml |
Adjusts the compiled cron minute for the every-12h schedule. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --- | ||
| description: Aligns code style with repository conventions, detects security issues, and improves tests | ||
| on: pull_request labeled refine | ||
| permissions: |
There was a problem hiding this comment.
The PR description focuses on cleaning actions/setup/js/validate_memory_files.cjs, but this PR also adds a new refiner workflow (and its generated lock), updates safe-outputs docs, tweaks a Go test, and changes a scheduled cron in an existing lock file. Please either update the PR description to cover these additional changes (and their motivation), or split the workflow/doc/test updates into separate PRs to keep scope reviewable.
| create-pull-request: | ||
| title-prefix: "[refiner] " | ||
| labels: [automation, refine-improvements] | ||
| reviewers: [copilot] |
There was a problem hiding this comment.
safe-outputs.create-pull-request.reviewers is configured here, but the generated .github/workflows/refiner.lock.yml doesn’t appear to include any reviewer-assignment steps/config (no reviewer-related entries in the safe-outputs handler config). If this workflow needs to automatically request reviewers, wire this through in the compiled workflow (or enable/use the add-reviewer safe output instead); otherwise consider removing reviewers: to avoid implying behavior that won’t happen.
| reviewers: [copilot] |
Summary
Cleaned and modernized
validate_memory_files.cjsfrom theactions/setup/js/directory following jsweep best practices.Context
This file runs in github-script context and validates that all files in a memory directory have allowed file extensions.
Changes Made
Code Improvements
?.) - SimplifiedallowAllcheck from!allowedExtensions || allowedExtensions.length === 0to!allowedExtensions?.lengthscanDirectoryfrom function declaration to arrow function for consistencyLine Count
Test Coverage
The file already has comprehensive test coverage with 21 test cases covering:
All tests pass successfully.
Validation Results
✅ Formatting:
npm run format:cjs- Passed✅ Linting:
npm run lint:cjs- Passed✅ Type checking:
npm run typecheck- Passed (2 pre-existing errors in other files, not in this file)✅ Tests:
npm run test:js -- --no-file-parallelism validate_memory_files- 21/21 tests passedBefore & After Example
Before:
After:
Impact