Repository navigation
Deduplicate memory allowed-extension parsing - #67543
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
clients2.google.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "clients2.google.com"See Network Configuration for more information.
💥 [THE END] — Illustrated by Smoke Claude · claude · sonnet46 · 72.2 AIC · ⌖ 9.55 AIC · ⊞ 853
Comment /smoke-claude to run again
| return entry, err | ||
| } | ||
| if err := parseCacheMemoryAllowedExtensions(cacheMap, &entry); err != nil { | ||
| allowedExtensions, err := parseMemoryAllowedExtensions(cacheMap) |
There was a problem hiding this comment.
👍 Good refactoring — using parseMemoryAllowedExtensions as a shared helper eliminates duplication. Consider adding a comment here explaining that this shared parser is also used by drive_memory_config.go to help future readers understand the coupling.
| entry.RestoreOnly = restoreOnly | ||
| } | ||
| if err := parseDriveMemoryAllowedExtensions(raw, &entry); err != nil { | ||
| allowedExtensions, err := parseMemoryAllowedExtensions(raw) |
There was a problem hiding this comment.
✅ Consistent with the change in cache_config.go. The shared parseMemoryAllowedExtensions function keeps both call sites in sync — if validation logic changes, only one place needs updating. Well done!
There was a problem hiding this comment.
🟢 Approval recommended
The refactor preserves both callers’ existing behavior and includes appropriate regression coverage.
0 open findings
What changed in this PR
Deduplicates memory allowed-extensions parsing while preserving existing behavior.
Changes:
- Adds a shared parser with validation tests.
- Migrates cache-memory and drive-memory parsing to the helper.
- Adds drive-memory regression coverage.
| File | Description |
|---|---|
pkg/workflow/memory_validation_config.go |
Adds shared extension parsing. |
pkg/workflow/memory_validation_config_test.go |
Tests parser behavior. |
pkg/workflow/cache_config.go |
Uses the shared parser. |
pkg/workflow/drive_memory_config.go |
Uses the shared parser. |
pkg/workflow/drive_memory_test.go |
Tests invalid extensions. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
🎉 This pull request is included in a new release. Release: |
Cache-memory and drive-memory duplicated parsing and validation for
allowed-extensions, risking inconsistent behavior as the rules evolve.parseMemoryAllowedExtensions.✨ PR Review Safe Output Test - Run 38098153730
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
clients2.google.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.