Add custom validation hooks for repo and cache memory#52053
Conversation
|
Thanks for starting this work! 🚀 This PR follows the project's agentic development process and aligns with the requirements in #52018. As a draft/WIP PR with no file changes yet, here are some pointers as implementation progresses:
The PR is on track. Once implementation begins and files start changing, reviewers will have substance to evaluate against the validation extension requirements from #52018.
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot the compiler update detector should flag changes to these script fields in the compile update security report |
|
🧠 Matt Pocock Skills Reviewer failed during 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 🏗️ failed during design decision gate check. 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 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.
|
|
❌ Ponytail Reviewer failed. Please review the logs for details. 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.
|
There was a problem hiding this comment.
Pull request overview
Adds configurable JavaScript validation hooks for repo-memory and cache-memory persistence.
Changes:
- Parses and documents validation scripts and timeouts.
- Runs validators before artifact upload, cache save, and repository commit.
- Adds focused compiler and runtime tests.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/safe_outputs_config_generation.go |
Passes validation configuration to safe outputs. |
pkg/workflow/repo_memory.go |
Generates repo-memory validation gates. |
pkg/workflow/repo_memory_test.go |
Tests repo-memory generation. |
pkg/workflow/memory_validation_config.go |
Parses shared validation configuration. |
pkg/workflow/compiler_github_actions_steps.go |
Removes obsolete step helper. |
pkg/workflow/cache.go |
Generates cache validation and save gates. |
pkg/workflow/cache_memory_syntax_test.go |
Tests cache validation generation. |
pkg/parser/schemas/main_workflow_schema.json |
Defines validation schema. |
docs/src/content/docs/reference/repo-memory.md |
Documents repo-memory validators. |
docs/src/content/docs/reference/cache-memory.md |
Documents cache-memory validators. |
actions/setup/js/safe_outputs_handlers.test.cjs |
Tests persistence-boundary validation. |
actions/setup/js/safe_outputs_handlers.cjs |
Runs repo-memory validators safely. |
actions/setup/js/push_repo_memory.cjs |
Revalidates before repository commit. |
actions/setup/js/memory_custom_validation.test.cjs |
Tests validator execution utilities. |
actions/setup/js/memory_custom_validation.cjs |
Implements validator execution and formatting. |
Review details
Tip
Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 15/15 changed files
- Comments generated: 4
- Review effort level: Balanced
| } | ||
|
|
||
| func cacheMemoryValidationStepID(cacheID string) string { | ||
| return strings.ReplaceAll("validate_cache_memory_"+cacheID, "-", "_") |
| } | ||
|
|
||
| func repoMemoryValidationStepID(memoryID string) string { | ||
| return strings.ReplaceAll("validate_repo_memory_"+memoryID, "-", "_") |
| const result = childProcess.spawnSync(process.execPath, [scriptPath], { | ||
| cwd: options.memoryDir, | ||
| encoding: "utf8", | ||
| env: sanitizedValidationEnv(process.env), | ||
| timeout: timeoutMs, |
| case float64: | ||
| return validateMemoryValidationTimeout(int(v), fieldPath) |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot Please inspect the latest branch state, resolve any already-addressed review threads that remain open, and use the Latest contributor update indicates the branch is green locally and prior feedback appears addressed, but reviewer threads may still need resolution by available tooling. Run context: https://github.com/github/gh-aw/actions/runs/31565195914
|
The branch remains clean and includes the current |
|
@copilot Please inspect the latest branch state, resolve any already-addressed review threads that remain open if tooling allows, and use the Latest contributor update indicates the cited reviewer findings are already addressed and the branch is green locally, but unresolved review threads still appear to be the remaining blocker. Run context: https://github.com/github/gh-aw/actions/runs/31567422879
|
I rechecked available resolution paths: |
Triage: pr-type:feature · pr-risk:high · Score 55/100 (impact 35, urgency 10, quality 10)Recommended action: defer Adds custom JS validation hooks for repo-memory/cache-memory persistence — large surface area (28 files, +1464/-200) spanning schema, compiler, safe-outputs, and docs. High risk due to size and new execution-context semantics (custom scripts at persistence boundary). CI green but only one reviewer comment so far; recommend deeper security/behavior review before merging.
|
|
@copilot Please inspect the latest branch state, resolve any already-addressed review threads that remain open, refresh the branch if needed, and run the Open review threads with author follow-up detected:
No failed checks were reported in the compact candidate set for this run.
|
Finishing pass complete: |
|
@copilot Please inspect the latest branch state, resolve any already-addressed review threads that remain open if tooling allows, and run the Open review threads with author follow-up detected:
No failed checks were reported in the compact candidate set for this run. Run context: https://github.com/github/gh-aw/actions/runs/31584431246
|
Verified again: the branch is clean, contains the current base SHA as an ancestor, and the four cited findings remain implemented (collision-free IDs, validator mutation detection, and integral timeout validation). Review-thread resolution is not exposed by the available GitHub tooling, so no resolution action is possible from this environment. |
|
@copilot Please inspect the latest branch state, resolve any already-addressed review threads that remain open if tooling allows, refresh the branch if needed, and run the Open review threads with author follow-up detected:
No failed checks were reported in the compact candidate set for this run. Run context: https://github.com/github/gh-aw/actions/runs/31588615979
|
Latest inspection confirms the branch is clean at |
|
@copilot Please inspect the latest branch state, refresh the branch if needed, and run the No failed checks were reported in the compact candidate set for this run. Run context: https://github.com/github/gh-aw/actions/runs/31594228868
|
|
@copilot Please inspect the latest branch state, resolve any already-addressed review threads that remain open if tooling allows, refresh the branch if needed, and run the Open review threads with author follow-up detected:
No failed checks were reported in the compact candidate set for this run. Run context: https://github.com/github/gh-aw/actions/runs/31596427052
|
Triage
Large diff (1464 additions / 200 deletions across 28 files) adding domain-validation hooks for repo/cache memory. High risk given size and lack of maintainer approval yet — defer for deeper review rather than fast-track.
|
repo-memoryandcache-memoryonly enforced generic storage limits, so structurally valid but domain-invalid state could be persisted and break later runs. Workflows can now define a per-memory JavaScript validator that runs at the persistence boundary.validation.scriptand optionalvalidation.timeoutto repo-memory and cache-memory entries.Runtime validation
format-jsonnormalization.false, nonzero exit, timeout, or missing script as persistence failures.Persistence gating
update_cache_memorybeforeactions/cache/save.Coverage
Run report: https://github.com/github/gh-aw/actions/runs/31554759452> Generated by 👨🍳 PR Sous Chef · gpt54 · 13.6 AIC · ⌖ 7.61 AIC · ⊞ 8.5K · ◷