fix(skills): charge only upstream waits to the install fetch budget - #80
Merged
Merged
Conversation
The skill install gave itself one four-second deadline, so writing each skill's files counted against it. On a slow disk the later skills gave up before asking upstream and reported the budget elapsed - even with DEVUP_MCP_SKILLS_OFFLINE set - which also made skills_install fail intermittently on Windows runners. The budget is now what is left of the time for waiting on upstream, and only that waiting is taken from it. The bridge handover tests now wait for the devup-mcp processes they start to exit before removing their scratch directories. Windows will not remove a directory a running process has as its current one, so every run left some behind.
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
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 release run for 0.13.0 failed once on Windows:
skills_install::a_bare_workspace_reports_the_gap_and_one_call_closes_itfound a skill installed with a reason that did not mentionDEVUP_MCP_SKILLS_OFFLINE, although the test sets it. A re-run passed. The flake had a real cause in the product.The cause
devup_skillsinstall gives itself a four-second budget for fetching current documents from upstream. It was one deadline for the whole call, so the time spent writing each skill's files to disk counted against it too. On a slow disk - a Windows runner - the deadline passed while earlier skills were being written, and the later skills gave up before asking upstream, reporting "the four-second skill install fetch budget elapsed" and falling back to the embedded copy. WithDEVUP_MCP_SKILLS_OFFLINEset that reason was simply wrong; online, it meant skills were never offered to upstream at all.The README already promises what the fix does: "호출 전체의 네트워크 대기는 최대 4초입니다" - at most four seconds of network waiting per call.
What changes
the_entire_install_has_one_four_second_fetch_budgetis unchanged and passes).time_spent_between_fetches_is_not_charged_to_the_fetch_budget: eight seconds pass outside any fetch, and the next skill still asks upstream and reports upstream's own answer. Against the old code it fails withNetwork("the four-second skill install fetch budget elapsed").skills_install's assertion now prints the offending entry, so a failure says which skill and why.%TEMP%; now none.Verification
cargo test --workspace120 suites, 1248 passed, 0 failed, 2 ignored;cargo clippy --workspace --all-targets --all-features -D warningsandcargo fmt --checkclean. Nodevup-bridge-handover-*directory is left after the handover tests.crates/devup-mcp/Cargo.tomlPatch, 0.13.0 -> 0.13.1.