[weiqingy]

Linked issue: none (hotfix)

Purpose of change

Two related cleanups in the skill module.

Repositories leaked on unexpected load failures. SkillManager released already-opened repositories only for a narrow set of exception types. Python's _load_skills guarded OSError and ValueError; Java's loadAll guarded IOException and IllegalArgumentException. Any other failure skipped the cleanup, so the repositories' temp directories and shutdown hooks leaked until interpreter or JVM exit. The caller never receives a SkillManager reference on a failed construction, so it cannot clean them up itself. Concrete cases: a corrupt archive raises zipfile.BadZipFile, which does not inherit from OSError; a failure inside _register_repo was outside the guarded region entirely; and on the Java side an unexpected RuntimeException or Error from open(), originOf(), or registerRepo() escaped the constructor.

The source loop is now wrapped in both languages so the cleanup runs on every failure path. Which exceptions get wrapped is unchanged: Python still wraps OSError and ValueError in RuntimeError, Java still wraps IOException and IllegalArgumentException in IllegalStateException, both with the same message and cause chaining, and everything else still propagates unchanged in type and identity. The only behavior change is that the cleanup now runs.

Java's guard takes Throwable, and so does the handler around the cleanup call itself. closeRepos() does not catch Error per repository and SkillRepository.close() declares no checked exception, so an Error raised while releasing a repository would otherwise escape and replace the original failure with nothing suppressed.

Dead code removed. SkillManager.resolveResourcePath and SkillRepositoryInfo have no callers. The resource-path method existed in both languages, so both are deleted in the same commit rather than leaving one side without its counterpart. Its only reference was a Java test, which is removed with it.

Tests

Four tests added, two per language, covering the previously unguarded paths.

Python TestSkillManagerMixedSources: a source raising zipfile.BadZipFile from open(), and a failure raised from _register_repo after an earlier source loaded cleanly. Both assert the earlier repository was closed and that the original exception propagates unwrapped.

Java SkillManagerTest: a repository failing registration with an IllegalStateException, asserting both the earlier repository and the failing one were closed, that the original exception arrives by identity, and that a cleanup failure arrives as suppressed. Identity rather than type is the load-bearing assertion, since IllegalStateException is also what the wrapping catch builds. A second test covers the Error path, which is what pins both handlers to Throwable.

All four were verified to be discriminating rather than merely green. On the Python side, re-narrowing the handler so it no longer spans _register_repo fails only the registration test while the pre-existing cleanup tests stay green. On the Java side, narrowing either handler back to Exception fails the Error test, and restricting addSuppressed to the wrapped path fails both new tests while the pre-existing suppressed-cleanup test passes.

Java SkillManagerTest 19 passed and the full runtime module 402 passed. Python skill module 68 passed and the full suite 619 passed, 11 skipped. spotless:check, ruff check, and ruff format are all clean.

API

No public API change. SkillManager is a runtime-internal component, and both removed symbols had no callers in either language.

Documentation

• doc-needed
• doc-not-needed
• doc-included

[wenjin272]

The Python path now cleans up repositories on every load/registration failure, but the Java SkillManager.loadAll() still catches only IOException | IllegalArgumentException. An unexpected RuntimeException or Error from open(), originOf(), or registerRepo() therefore still escapes without calling closeRepos(). I reproduced this with repo.getSkills() throwing IllegalStateException: neither the earlier repository nor the repository whose registration failed was closed. Could we add the equivalent Java cleanup path, preserve the original exception with cleanup failures suppressed, and cover it with a regression test?

[weiqingy]

Good catch, that gap was real. I reproduced it before fixing: an IllegalStateException out of registerRepo left both the earlier repository and the failing one open.

Fixed in 59344b0. The source loop is now wrapped in an outer guard that runs closeRepos() on every failure path, attaches any cleanup failure as suppressed, and rethrows the original itself. Which exceptions get wrapped in IllegalStateException is unchanged: IOException and IllegalArgumentException still wrap with the same message and cause, and everything else still propagates with the same type and identity. The only behavior change is that the cleanup now runs, which matches what the Python side does.

The handler around the cleanup call itself was also too narrow. closeRepos() catches only Exception per repository, and SkillRepository.close() declares no checked exception, so an Error from a repository's close() escaped and replaced the primary failure with nothing suppressed. Both handlers now take Throwable.

Two regression tests. The first is your scenario: getSkills() throws IllegalStateException, and it asserts both repositories were closed and that the original exception reaches the caller by identity. Identity rather than type matters here, because IllegalStateException is also what the wrapping catch builds, so an assertThrows on the type alone would pass either way. The second covers the Error half you named. Nothing else distinguishes catch (Throwable) from catch (Exception), so a future narrowing would otherwise build green.

runtime 402 passed, spotless:check clean.

[wenjin272]

LGTM