fix(config-sync): isolate global capabilities in sync tests - #842
Merged
Merged
Conversation
Run the two-device scenario in its own process with a temporary agents root. This prevents capturing developer skills and avoids races with tests that mutate the process-wide capability directory.
Include the new WebDAV compatibility work before candidate validation. Preserve the original PR behavior and both upstream and author history.
The latest main adds another sync scenario that captures global skills. Reuse the isolated child-process runner for both scenarios so the new append-only test cannot read developer skills or race with other tests.
yuxino
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The configuration-sync tests can capture global skills from the developer's real
~/.agentsdirectory despite using temporary databases. A local skill resource larger than 128 KiB makes the tests fail withSKILL_LIMIT_EXCEEDED. SettingPI_DESKTOP_AGENTS_DIRfor the whole suite is insufficient because parallel tests temporarily replace and remove it.Run both the two-device sync scenario and the newly added append-only compatibility scenario in exact-filtered child test processes, each with its own temporary agents directory. Both use the same test-only runner. Existing sync assertions and production size limits remain unchanged.
Validation: reproduced the missing isolation in the append-only scenario after incorporating current main, then ran the full Host suite successfully (623 tests),
cargo fmt --check, and Clippy. The five engine scenarios also passed on the committed candidate. Clippy retains the existinguser_skills.rsuseless-format warning.Candidate
a4a0a68c64df3a45d01ae4baRecorded terminal test evidence
pr-842-terminal.mp4
This is a recording of actual PTY output, rendered with asciinema agg and encoded as video, not a native Terminal window capture or a product UI demonstration. The raw timestamped asciicast v2 recording is attached as
.txtfor inspection. Playback is uniformly 1.5× speed; the final result has an added reading hold. No test output was rewritten.The same synthetic temporary agents directory (one 131073-byte skill resource) is supplied to four actual test executions against local HTTP WebDAV fixtures. On upstream
3a45d01ae4ba, both the two-device and append-only scenarios fail withSKILL_LIMIT_EXCEEDED/ exit 101. On candidatea4a0a68c64df, both pass / exit 0 because their child processes isolate the global capability root. No real user skills, secrets, or production endpoints are used. Production limits remain unchanged.The 2260 × 1286 video demonstrates the ambient-directory failure and both isolated successes. It does not attempt to reproduce the separate parallel environment-variable race on screen.