Various Bug Fixes (Undo, Pool Auto-Purge)#247
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens reliability across Unity editor tooling (Undo safety), runtime pooling (auto-purge correctness/perf), and contributor automation (new validators, hook hardening, and changed-file preflight) to prevent regressions from slipping through hooks/CI.
Changes:
- Adds a lint-error-code ↔ cspell contract validator (plus CI workflow + regression tests) to ensure new
^[A-Z]{2,}\d{3}$code families don’t break spelling checks. - Introduces/expands contributor automation:
agent-preflight(changed-file validation + safe fixes), hook hardening (pre-merge-commit delegation, staging retry diagnostics), and new lint/test coverage around PowerShell invocation safety and git-staging helpers. - Fixes pooling auto-purge throttle edge cases (capacity enforcement + timestamp max-semantics) and improves editor Undo behavior + corresponding tests/docs/perf snapshots.
Reviewed changes
Copilot reviewed 78 out of 78 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/validate-lint-error-codes.ps1.meta | Adds Unity .meta for new validator script. |
| scripts/validate-lint-error-codes.ps1 | New contract validator to ensure lint-error-code prefixes roundtrip through cspell. |
| scripts/validate-hook-sync-calls.ps1 | Adds validation that pre-merge-commit delegates to pre-commit. |
| scripts/validate-devcontainer-urls.sh | Adds --contracts-only, absolute Dockerfile support, and contract to reject apt-based PowerShell install. |
| scripts/tests/test-validate-lint-error-codes.ps1.meta | Adds Unity .meta for new validator test. |
| scripts/tests/test-validate-lint-error-codes.ps1 | New test suite for lint-error-code validator behavior + fixtures. |
| scripts/tests/test-validate-devcontainer-urls.sh | Extends contracts to cover new apt-PowerShell rejection logic. |
| scripts/tests/test-sync-script-contracts.ps1 | Adds agent/pre-push spelling contract checks. |
| scripts/tests/test-precommit-integration.sh.meta | Adds Unity .meta for new integration test script. |
| scripts/tests/test-lint-skill-sizes.ps1 | Extends test harness to pass additional args; adds new scoped/critical-mode tests. |
| scripts/tests/test-lint-pwsh-invocations.ps1.meta | Adds Unity .meta for new PowerShell invocation lint tests. |
| scripts/tests/test-lint-dependabot.ps1 | Updates test invocation to use pwsh -File ... -Paths (production-equivalent). |
| scripts/tests/test-git-staging-helpers.sh.meta | Adds Unity .meta for new git staging helper regression tests. |
| scripts/tests/test-git-staging-helpers.sh | Adds regression tests for stderr preservation and trap leakage behavior. |
| scripts/tests/test-agent-preflight.ps1.meta | Adds Unity .meta for new agent-preflight tests. |
| scripts/tests/test-agent-preflight.ps1 | Adds synthetic-repo tests for agent-preflight (meta/spelling/lock contention). |
| scripts/lint-skill-sizes.ps1 | Adds -Paths scoping + -FailOnCritical behavior and context-check gating. |
| scripts/lint-pwsh-invocations.ps1.meta | Adds Unity .meta for new PowerShell invocation linter. |
| scripts/lint-dependabot.ps1 | Adds -Paths + remaining-args binding pattern; improves LiteralPath usage and multi-path handling. |
| scripts/install-hooks.sh | Ensures pre-merge-commit hook exists and is made executable. |
| scripts/install-hooks.ps1 | Adds status/install checks for pre-merge-commit hook. |
| scripts/git-staging-helpers.sh | Fixes cleanup to avoid persistent stderr redirection; explicit cleanup calls. |
| scripts/git-staging-helpers.ps1 | Adds env-tunable lock retry knobs + safer jitter calculation. |
| scripts/agent-preflight.ps1.meta | Adds Unity .meta for new agent-preflight script. |
| scripts/agent-preflight.ps1 | New changed-file preflight validator (meta coverage, spelling on changed markdown, etc.). |
| package.json | Adds agent/preflight scripts and wires new validators/tests into validation pipelines. |
| docs/performance/relational-components-performance.md | Updates benchmark snapshot table/date. |
| docs/performance/random-performance.md | Updates benchmark snapshot values and references. |
| docs/performance/ilist-sorting-performance.md | Updates benchmark snapshot table/date. |
| cspell.json | Adds new project words/prefixes (e.g., PWS) used by new lint/skills. |
| Tests/Runtime/Utils/BuffersTests.cs | Ensures PoolPurgeSettings reset for test isolation. |
| Tests/Runtime/Pool/TypeAwarePoliciesTests.cs | Disables MemoryPressureMonitor for determinism; restores state in teardown. |
| Tests/Runtime/Pool/PoolSizeAwarePoliciesTests.cs | Disables MemoryPressureMonitor for determinism; restores state in teardown. |
| Tests/Runtime/Pool/PoolPurgeStrategyTests.cs | Tightens hysteresis/retain tests; clarifies intent; adjusts parameters for determinism. |
| Tests/Runtime/Pool/PoolAccessFrequencyTests.cs | Adds PoolPurgeSettings resets in multiple fixtures for isolation. |
| Tests/Runtime/Pool/GradualPurgingTests.cs | Disables/restores MemoryPressureMonitor for determinism. |
| Tests/Runtime/Pool/GlobalPoolRegistryTests.cs | Adds PoolPurgeSettings reset and MemoryPressureMonitor save/restore. |
| Tests/Runtime/Performance/PoolPurgeTriggerPerformanceTests.cs | Clarifies performance test intent around throttle fast path. |
| Tests/Editor/Sprites/TextureSettings/TextureSettingsApplierAPITests.cs | Adds Undo regression test for texture settings application. |
| Runtime/Utils/Buffers.cs | Fixes auto-purge throttle correctness (capacity bypass) + timestamp advancement semantics. |
| Editor/Tags/AttributeMetadataCacheEditor.cs | Makes cache purge undoable and ensures asset dirtied/saved. |
| Editor/Sprites/TextureSettingsApplierAPI.cs | Adds Undo recording on importer mutations (single-record pattern). |
| Editor/Sprites/SpriteSettingsApplierAPI.cs | Adds Undo recording on importer/settings mutations (single-record pattern). |
| Editor/Sprites/SpritePivotAdjuster.cs | Records Undo before importer setting mutation. |
| Editor/Sprites/SpriteCropper.cs | Switches to ApplyModifiedProperties (Undo-aware) and correct modified flags. |
| Editor/Sprites/ScriptableSpriteAtlasEditor.cs | Adds Undo recording for importer/platform setting mutation batch. |
| Editor/FitTextureSizeWindow.cs | Records Undo before changing importer max texture size. |
| Editor/CustomEditors/PolygonCollider2DOptimizerEditor.cs | Records Undo and sets dirty on affected objects. |
| Editor/CustomEditors/MatchColliderToSpriteEditor.cs | Records Undo and sets dirty on component/collider. |
| CHANGELOG.md | Documents pooling throttle fix + performance improvements + editor drawer fixes. |
| .llm/skills/validate-before-commit.md | Updates guidance to require agent preflight before completion; updates validation steps. |
| .llm/skills/manage-skills.md | Adds agent-preflight to skill maintenance workflow. |
| .llm/skills/linter-reference.md | Documents new lint/contract scripts and policy for lint-error-code prefixes. |
| .llm/skills/investigate-test-failures.md | Adds guidance on virtual-clock throttles and pool fixture hygiene. |
| .llm/skills/git-hook-syntax-portability.md | Documents pwsh -File -- caveat and points to bash→pwsh invocation skill. |
| .llm/skills/formatting-and-linting.md | Updates hook behavior docs (spelling patch, pre-merge-commit delegation). |
| .llm/skills/editor-undo-complete.md | New skill documenting undo tier policy and required patterns. |
| .llm/skills/create-unity-meta.md | Adds agent-preflight step to meta creation checklist. |
| .llm/skills/bash-pwsh-invocation.md.meta | Adds Unity .meta for new skill doc. |
| .llm/skills/bash-pwsh-invocation.md | New skill defining safe bash→PowerShell -File invocation rules + enforcement. |
| .llm/context.md | Regenerates skills index and updates repo rules around preflight and undo policy. |
| .github/workflows/validate-hooks.yml | Adds precommit integration smoke test step and pwsh availability precondition. |
| .github/workflows/validate-devcontainer.yml | Adds live URL validation step in devcontainer workflow. |
| .github/workflows/pwsh-invocations-lint.yml.meta | Adds Unity .meta for new workflow file. |
| .github/workflows/pwsh-invocations-lint.yml | New workflow to run PowerShell invocation lint + tests. |
| .github/workflows/lint-error-codes.yml | New workflow to enforce lint-error-code ↔ cspell contract in CI. |
| .github/ISSUE_TEMPLATE/feature_request.yml | Adds 1.0.1 to version list. |
| .github/ISSUE_TEMPLATE/bug_report.yml | Adds 1.0.1 to version list. |
| .githooks/pre-push | Adds lint-error-code contract conditional runs + improved spelling diagnostics/patch output. |
| .githooks/pre-merge-commit | New hook delegating merge commits to pre-commit to close validation gap. |
| .githooks/pre-commit | Improves hook robustness (pipefail, staging diagnostics, safe pwsh invocation patterns, scoped lint-skill-sizes, better cspell capture/patch). |
| # Share node_modules with the real repo so `npx --no-install cspell` works | ||
| # inside the fixture CWD without re-downloading dependencies. A symlink is | ||
| # safe here because the validator only reads node_modules — it never | ||
| # writes. On platforms where symlink creation is forbidden (Windows non- | ||
| # admin, some CI runners), fall back to a package.json that points npm at | ||
| # the repo's node_modules directory via NODE_PATH. We prefer the symlink | ||
| # path because `npx --no-install` resolves through standard Node module | ||
| # resolution rules, which follow symlinks transparently. | ||
| $fixtureNodeModules = Join-Path $root 'node_modules' |
There was a problem hiding this comment.
The fallback path for making npx --no-install cspell work in the fixture is internally inconsistent: the comment says it will fall back to a package.json/NODE_PATH approach when symlinks are forbidden, but the implementation actually does a filesystem copy. Either implement the described NODE_PATH/package.json fallback, or update the comment to match the real behavior so future maintainers don’t assume a lightweight fallback exists.
| catch { | ||
| # Fallback: copy just the cspell bin + hoisted dirs. We keep this | ||
| # narrow to avoid ballooning the fixture. | ||
| Write-Info "Symlink creation failed ($_); falling back to copy." | ||
| Copy-Item -LiteralPath $realNodeModules -Destination $fixtureNodeModules -Recurse -Force -ErrorAction SilentlyContinue | ||
| } |
There was a problem hiding this comment.
When symlink creation fails, the test currently does Copy-Item -Recurse of the repo’s entire node_modules into the fixture. That can be extremely slow and disk-heavy (especially on Windows) and contradicts the intent to keep the fixture small. Consider switching this fallback to a lightweight approach (e.g., set NODE_PATH to the real repo’s node_modules for the validator process, or copy only the minimal cspell-related subtree needed for npx --no-install cspell).
Description
This PR delivers a set of focused quality and reliability updates across editor tools, runtime pooling performance, and project automation.
Key user-facing improvements:
Project quality improvements:
Related Issue
Fixes #
Type of Change
Checklist