fix(upm): unload uninstall targets through PluginManager#unregister and report real outcomes (#503, #501, #505)

[wisdommen]

Summary

/upm uninstall unloads through the framework's one full unload path and reports what actually happened on disk; /upm update installs a new version and lets the next start confirm or roll it back, rather than predicting whether the module will load. Companion to #500 (#495) in the same Phase 17 wave-0 real-machine session.

#503: /upm uninstall bypassed PluginManager#unregister. PluginInstallUtils#uninstallPlugin called plugin.unregisterSelf() directly, so a module's @Scheduled tasks, @PlayerCache beans, completers, EventBus handlers, panel responders and @ConditionalOnConfig records outlived the uninstall and its container was never closed. It now unloads every matching module through PluginManager#unregister and delists it.

• If the module's own unload throws, the module is still delisted and deletion of its jars is still attempted. The failure is logged and reported together with the jar outcome instead of escaping as a generic command error. Keeping the jar would only bring back on restart a module the operator asked to remove; by then its container is already closed.

#501: the uninstall reply did not match the outcome. The success reply told the operator to delete a jar the command had already deleted, and the delete() result was ignored. The reply now reflects the real outcome:

• success only once every jar whose plugin.yml name: matches is deleted (not just the first one);
• a red failure naming every jar still on disk;
• a yellow notice when a loaded module was unloaded but no jar was found, instead of "check the spelling";
• a jar the command could not read is reported rather than passed over, since it may be a copy of the module that loads it again once the file is readable: every unreadable jar when none could be identified, and the ones named like this module when its jar was found and deleted. An unrelated unreadable file still does not stop an uninstall.

#505: /upm update reported success for updates it did not complete. The update is a staged transaction whose last step is the next start:

1. an update or uninstall of the same module that is already running refuses the new one, and nothing changes (one guard covers /upm update and /upm uninstall, keyed by identify-string and by the module's runtime name);
2. the update is refused before anything is downloaded unless the staging directory and the modules folder are on the same file system — a jar is only ever renamed atomically, never copied, so a crash cannot leave a truncated jar under its final name;
3. the new version is downloaded into plugins/UltiTools/.upm-staging/ under a unique name that does not end in .jar, outside the modules folder;
4. it is validated against the module's plugin.yml contract — identify-string, name:, and the catalogue's latest version, compared as written so 2.10 stays 2.10;
5. the module's older jars are selected from the modules folder, where the new version does not yet exist; if any is newer than the catalogue version the update is refused and nothing changes; otherwise the transaction is journaled and they are moved aside;
6. the new jar is moved in under its final name (on failure, the older jars are restored);
7. the transaction stops there. The old jars stay in staging, the journal stays with them marked awaiting-boot-confirmation, and the reply says the restart confirms the update.

The next start completes it. After the modules have loaded, every journal awaiting confirmation is resolved by observing what happened. The question is asked of the module's identify-string, which the journal records — two modules can carry the same plugin.yml name:, and the one that loaded need not be the one this journal updated:

• the module is among the loaded modules → the old jars and the journal are deleted;
• it is not → the installed jar is deleted, the old jar is moved back where it came from, and a SEVERE console line names the module, the version restored, that the module is not available in this session, and that a restart loads it again. A rollback with more than one old jar to put back keeps its journal until every one of them has settled, so a pair it could not place is retried at the next start instead of being stranded in staging.

If the journal cannot be marked as awaiting a boot — the disk fills, permissions change — the update undoes itself there and then, while both versions are still on disk, and reports a failure: without that record nothing could confirm or roll it back, and the reply had just promised the restart would.

The restored module is deliberately not registered in flight — registering after the load phase has its own hazards, and the restart boundary is stated rather than implied. A failure anywhere in this hook is logged and never stops the server, which is pinned by a test.

Why the load prediction was deleted. Earlier revisions of this branch tried to answer, before installing, whether a module would load from a download — first with a class-file parser, then with the loader's own scan run in a reconstructed environment. Nineteen review findings in a row were each one difference between that answer and what boot does. Two of them say why the shape cannot work:

• boot URL ordering: UltiTools#collectModuleJarUrls builds the class path from File.listFiles, which guarantees no order. There is no boot ordering to reproduce — the thing being predicted is not fixed.
• recovery target loadability: the validator structurally cannot run where recovery needs it. It logs through Bukkit.getLogger() and opens a class loader, and boot recovery runs before either exists; measured, adding the call made two recovery tests fail with nothing restored.

Boot confirmation does not ask whether a module will load. It observes whether it did, so that class of divergence has nothing left to attach to. What remains of validation is the plugin.yml contract, which is finite and documented and has nothing to diverge from.

The durable-state enumeration

The update's state on disk is finite, so it is enumerated here once rather than one row per review round. Three tables: every gap between two consecutive durable writes, every operation an operator can run while a journal is unresolved, and every status the new code receives and drops. Twelve rows were wrong and are fixed in this pull request — eight found by the enumeration itself, and four the confirmation reviews corrected: two verdicts the enumeration got wrong (A12, C7), one window it read wrongly (A18), and one row it did not have at all (A21, a retried rollback). Each is pinned by a test that fails when the fix is reverted in a throwaway copy.

The durable writes, in execution order. An update: W1 create .upm-staging/ · W2 write the download as <id>-<version>-<uuid>.part · W3 create the journal (temp file, then rename) · W4 move older JAR k aside (k = 1..N) · W5 move the download in as <id>-<version>.jar · W6 rewrite the journal with phase=awaiting-boot-confirmation. At the next start, confirmation: W7 delete set-aside JAR k · W8 delete the journal; or rollback: W9 rewrite the journal with phase=rolling-back · W10 delete the installed JAR · W11 move set-aside JAR k back · W12 delete the journal. An uninstall: W13 delete the module's set-aside JARs and journals · W14 delete the module's JARs.

Table A — crash windows

#	Process dies	Pre-load recovery sees	Confirmation sees	Verdict
A1	before W1	nothing	—	correct
A2	inside W2 (partial download)	a .part file; deletes it	—	correct
A3	W2→W3	a complete .part; deletes it. The modules folder was never touched	—	correct
A4	inside W3 (temp written, not renamed)	a .txn.tmp; deletes it	—	correct
A5	W3→W4	a journal naming JARs that were never moved; nothing to restore, journal deleted	—	correct
A6	inside W4 (k of N moved)	restores the k that moved; the rest were never moved; journal deleted	—	correct
A7	W4→W5 (all aside, nothing installed)	restores every set-aside JAR; the module loads its old version	—	correct
A8	W5→W6 (installed, journal unmarked)	was: called the set-aside JARs leftovers of a finished update and deleted the journal — so an unloadable new version could never be rolled back	nothing: the journal was already gone	wrong — fixed. An installed target plus a journal of a dead process is handed to confirmation exactly as a marked journal is
A9	after W6	leaves it	decides it	correct — this is the design
A10	inside W7 (some set-aside JARs deleted)	leaves it	the module loaded; deletes the rest and the journal	correct
A11	W7→W8	leaves it	confirms; journal deleted	correct
A12	after W9, with the rollback not yet done	was: left for the confirmation hook, which runs after the modules load — so the rejected JAR sat on the class path for that session and the JAR that works was put back only afterwards, costing the module a further restart	—	wrong — fixed (Codex confirmation round). A rolling-back journal carries a verdict made at an earlier start and needs nothing from the load phase, so pre-load recovery finishes it; the class path is built after the restore
A13	W10→W12 after ≥1 JAR was restored	finishes the rollback (row A12)	was: the restored JAR loads the module, so confirmation read it as an update to confirm and deleted what the rollback still owed	wrong — fixed. phase=rolling-back is written before the rollback touches anything, and a start that finds it finishes the rollback rather than confirming it
A14	after W11, before W12	leaves it	finishes: nothing left to restore, journal deleted, the restored version named	correct
A15	a rollback with no set-aside JARs at all (a first-time install, or a module whose own JAR was gone)	—	was: kept the journal and repeated the same SEVERE at every start with nothing left to do	wrong — fixed. The journal goes; the operator is told the module is not installed
A16	the in-transaction rollback when the marker cannot be written	—	—	wrong — fixed. It restored the old JARs while the candidate still held the target path — a same-version retry sets aside a JAR whose path that is. The candidate is deleted first, and the journal is kept if a JAR stays in staging
A17	an uninstall, between W14 and W13	a journal that restores a JAR of the module just removed — the module comes back	—	wrong — fixed. The staging state is cleared first (W13 before W14), so a crash leaves the module installed, which is where it started
A18	inside the journal rewrite on a provider that refuses an atomic replace	was: the replacement is written under a temporary name, so a crash between removing the old journal and renaming the new one in left a complete journal with none beside it — and recovery deleted it as stale, leaving the set-aside JARs unnamed and an unloadable candidate unrollbackable	—	wrong — fixed twice. Files.move leaves replace-on-ATOMIC_MOVE to the provider, so one that refuses is handled by removing the target and retrying the rename; and a .txn.tmp with no journal beside it is now adopted when it is complete, deleted only when it is half-written. The first fix's own window is closed by the second (Codex confirmation round)
A19	inside W13 (some staged files deleted)	the module is still installed; a surviving journal may restore an older JAR	—	accepted: nothing is lost and the uninstall is re-runnable
A21	a rollback retried, where a set-aside JAR's own path is the target (a same-version retry)	finishes it	—	wrong — fixed (Codex confirmation round; the enumeration had no row for a retried rollback). The first attempt restores that JAR onto the target path and its staging copy is gone; a second attempt that deletes whatever sits there takes the module's only copy. The pairs and the staging directory say which case it is
A20	inside W14 (k of N JARs deleted)	the remaining JARs still load the module; the staging state is already clear, so nothing restores anything	—	accepted: re-run the uninstall

Table B — what is legal while a journal is unresolved

#	Operation	Outcome	Verdict
B1	/upm update of the same module	was: accepted — a second transaction set the first candidate aside and left two journals awaiting one restart; the one resolved second could restore the candidate the first rejected	wrong — fixed. Refused with ALREADY_IN_PROGRESS, nothing downloaded and nothing changed
B2	/upm update of a different module	accepted; each journal names its own module and is resolved on its own identify-string	correct
B3	/upm update all	the module with the unresolved journal is counted among the skipped; the others run	correct
B4	/upm uninstall of the same module	the module's journals and their set-aside JARs are deleted with its JARs, so nothing awaits a restart	correct
B5	/upm install of the same module	not guarded: the installed JAR occupies the target path, and the restart then confirms or rolls back whatever is there	accepted — installing a module that already has an update pending is answered by the same restart
B6	/ul reload	configs are re-read; no JAR moves, and the confirmation hook runs only at start	correct
B7	a clean restart	pre-load recovery leaves the journal; the modules load; confirmation decides	correct — this is the design
B8	a crash restart	identical to B7: the journal is process-independent	correct

Table C — signals the code receives and drops

#	Signal	Caller	Verdict
C1	journalPairs' incomplete-read flag	confirmOneUpdate passed a throwaway AtomicBoolean	wrong — fixed. A journal recording a set-aside JAR past a gap is kept by both the confirm and the rollback path
C2	the same flag	leaveForBootConfirmation	accepted: a pair it could not read is not marked accounted for, so the leftover pass names that JAR as belonging to a kept journal, which is the report that case needs
C3	deleteStagedFile's failure	confirmUpdate logs it and still deletes the journal	accepted: the update is confirmed, and the file is outside the modules folder and never loads
C4	markJournalRollingBack's failure	logged at SEVERE, the rollback proceeds	accepted: leaving a module that cannot load installed is worse; what is lost is only row A13's protection
C5	Files.deleteIfExists' boolean at five sites (stale download, temp journal, journal, installed target, staged download)	discarded	accepted: "already gone" is the outcome asked for; a failure arrives as an exception
C6	the same in deleteAllOrThrow	now logged when it deletes	the log is also what makes row A17's order observable
C7	a journal that cannot be read while checking whether an update owns this module	was: skipped, so the update started beside it	wrong — fixed (Codex confirmation round). Nothing can show such a journal is not this module's awaiting update, and starting beside it produces the two-verdict boot row B1 exists to prevent; the update is refused and the unusable staging state reported by path
C8	restoreSetAsideJar, recoverPair, markJournalAwaitingBoot, newVersionWasInstalled	every caller uses the value	correct
C9	confirmUpdatesAfterBoot returns void	UltiTools#initPluginModules	accepted by design: a boot barrier that must not fail the server

Two rulings applied

• A suspect JAR is reported whether or not a live instance was unloaded. A module that failed to load this boot has no instance, and its unreadable second copy is no less able to load it later. Reporting is not refusing: an unrelated unreadable file still does not stop an uninstall, because only JARs named like this module count as suspects once the metadata cannot be read.
• The journal rewrite does not rely on ATOMIC_MOVE replacing an existing target (row A18), and a provider that refuses it is a test, not a comment.

Behaviour change for operators. /upm update now replies Update installed! Please restart the server: the restart confirms whether the module loads, and rolls back to the previous version if it does not. — and the restart may roll the update back, saying so on the console. FEATURES.md carries the new ultitools.upm.update.boot-confirmation row; UAT-CHECKLIST.md carries ultitools.upm.update.boot-confirms and ultitools.upm.update.boot-rolls-back.

Compatibility. No existing public signature changes; one method and its result type are added (PluginInstallUtils.updatePluginTransactionally(String) returning PluginInstallUtils.UpdateOutcome). COMPATIBILITY.md records two changes under "Behavioral changes that need no migration period", as corrections of behaviour that contradicted the methods' own javadoc:

• uninstallPlugin now throws FileSystemException (a jar that could not be deleted), NoSuchFileException (module unloaded, no jar) or IllegalStateException (the module's unload threw).
• uninstallPlugin also throws PluginModuleException with the new ErrorCode.PLUGIN_OPERATION_IN_PROGRESS, with nothing changed, while an update or uninstall of the same module is running. The module's keys are resolved from the loaded modules, from the jars in the modules folder, and from the journals of running updates — in the window between moving the old jar aside and moving the new one in, the journal is the only thing that names the module — and they are taken all-or-none under one monitor.
• updatePlugin returns false only when nothing changed (failed download, staging unavailable or on another file system, invalid download, newer jar present, refused concurrent operation), and throws UncheckedIOException wrapping a FileSystemException — with the move's exception as cause — when a move fails or a set-aside jar could not be moved back.
• updatePluginTransactionally, UpdateOutcome and the boot-time recoverInterruptedUpdates are @ApiStatus.Internal. On Windows a loaded module's jar cannot be moved, so the update rolls back and says so.

Reverted within this branch. Two commits (098be913, d0e67f13) also routed the superseded-version unload in PluginManager#unregisterSupersededVersions through unregister(existing); e7b5abad and 79639271 revert them. The independent review showed that change also stripped the newer copy's name-keyed completers, responders and EventBus handlers, because those registries are keyed by the module name both copies share. That leak and the registry collision are tracked as #506 (wave 4); the path is reachable only from code. PluginManager.java is unchanged from alpha. The reason is also recorded on #503: #503 (comment)

Routed, not fixed here:

• /upm uninstall aborts on any non-jar entry in the modules folder, after the module has already been unloaded #504: uninstallPlugin still builds a jar:file: URL string. It fails on a non-jar entry, and (new trigger added to the issue) on a server path containing a space.
• Superseded module version is unloaded with unregisterSelf() only, and PluginManager#unregister cannot replace it while registries are name-keyed #506: the superseded-version unload.
• PluginManager#unregister leaves the module in pluginList, so every caller must delist it by mutating the live internal list #507: unregister() does not delist, the async /upm list races the list, and (comment added) module config entities stay registered and are saved at shutdown after uninstall.

Doc-sync: not owed. Measured on UltiTools-Dev-Doc origin/alpha (e02aa7e): git grep -i -c -E "uninstall|upm update|PluginInstallUtils" -- docs/src returns no hits. The only /upm mentions (git grep -i -c "upm ") are the upm list / upm install prose in guide/advanced/maven-plugin.md and its zh copy. Control: git grep -c UltiToolsPlugin returns hits, so the tree was read.

中文摘要：/upm uninstall 改为通过 PluginManager#unregister 完整卸载（#503），并如实报告结果：所有同名 JAR 删除后才报成功，删除失败时逐一列出残留 JAR，模块已卸载但找不到 JAR 时单独提示，模块自身卸载抛异常时同时报告 JAR 结果（#501）。/upm update 改为分阶段事务：同一模块的并发更新被拒绝；先下载到模块目录外的暂存目录并校验，再选出并移走旧 JAR、移入新 JAR，任何失败都会回滚且如实报告（#505）。之前在原地写入后再猜测旧 JAR 的做法连续四次修补都引出相邻缺陷，故整体重做。COMPATIBILITY.md 记录了两个方法的行为变化。分支内曾修改“被取代版本卸载”路径，因会误删新版本按名称登记的补全器等而已回退，转入 #506；#504、#507 另行跟踪。第四轮审查后：暂存目录与模块目录不在同一文件系统时拒绝更新（只做原子重命名，不复制），启动时恢复中断更新留下的旧 JAR，更新与卸载共用同一模块锁，拒绝覆盖比目录版本更新的 JAR，失败原因写入回复。无需同步开发者文档。第五轮审查后：更新事务在移动 JAR 前写入日志文件，启动恢复只依据日志回移其记录的 JAR，无日志的残留 JAR 只报告不回移（因此已卸载的模块不会复活，崩溃也不会把模块降级到旧版本）；日志记录写入它的 JVM，因此本进程仍在进行的事务不会被自行恢复；卸载的模块锁也从日志解析，并一次性全部获取；「未做任何更改」不再与随后列出的残留文件自相矛盾；跳过统计的中文措辞改为「另一项更新或卸载」。

Issue closure

Closes #503
Closes #501
Closes #505

Verification

Run in the worktree at the final commit (aa739534), JDK 21, every log written after that commit:

• mvn -B -nsu clean verify: Tests run: 5876, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, japicmp gate passed (all entries for the touched classes are additions: ErrorCode.PLUGIN_OPERATION_IN_PROGRESS, updatePluginTransactionally, recoverInterruptedUpdates, UpdateOutcome). The POSIX-permission, symbolic-link and hard-link tests ran and were not skipped.
• mvn -B -nsu test -Dgroups=isolated -DexcludedGroups=: Tests run: 23, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.
• .github/scripts/check-cjk-scope.sh: 0 CJK violation(s) across 0 file(s).
• PMD 6.55's design.xml NPathComplexity rule (threshold 200) over src/main: 0 violations.

Review rounds. Eight Codex rounds and six own-review rounds; every thread is answered (unreplied threads: 0) and every review file's dispositions are filled. The later rounds hardened the recovery path: a restart-unique journal owner, a recorded committed phase, journals retained while anything they name is unresolved, leftovers never advertised as deletable while their journal survives, downloads validated against what the loader actually requires, and an uninstall that clears — and reports — the staging state that could otherwise resurrect the module.

Static analysis. Codacy reports Your pull request is up to standards! and CodeQL No new alerts in code changed by this pull request. Both were red on the previous heads of this branch and are now clear:

• CodeQL's one critical alert was the raw-scalar plugin.yml reader added in review round 4, which built its parser with SnakeYAML's default constructor. It only composes a node tree, so no object is ever constructed from a module's file, but the parser is now built with SafeConstructor — measured to exist with a LoaderOptions parameter in every SnakeYAML from 1.26 to 2.2, so no runtime fallback is needed.
• Codacy's 13 findings were three NPathComplexity methods (51,840 / 4,800 / 340 against a threshold of 200), six fields declared below the methods that use them, two fully qualified names with an import already present, one Semgrep path-traversal hit on a test reading its own temporary directory, and one markdown code span with a trailing space inside it. The complexity was resolved by naming each step of the transaction and of recovery as its own method, not by suppression; the rollback sequence that existed in four copies is now one method.

Red-when-reverted evidence

The work was test-first; no fix commit touches a test file. For each fix, the named tests fail with the fix reverted (assertion failures, not compile errors) and pass with it restored:

• /upm uninstall bypasses PluginManager#unregister: scheduled tasks, EventBus handlers and the module context outlive the uninstall #503 uninstall unload path (cb44d44e); /upm uninstall tells the operator to delete the module jar manually after it has already deleted it #501 reply and delete result (edba829f); /upm uninstall tells the operator to delete the module jar manually after it has already deleted it #501 every jar (7478be55); /upm uninstall tells the operator to delete the module jar manually after it has already deleted it #501 unloaded module with no jar (5b9c381e); /upm uninstall bypasses PluginManager#unregister: scheduled tasks, EventBus handlers and the module context outlive the uninstall #503 uninstall whose module unload throws (8ed656c5).

• /upm update reports success when the old module jar could not be deleted (updatePlugin ignores File#delete) #505 staged update transaction (fd384224): an invalid download (invalidDownload_keepsTheOldJarAndReportsFailure, downloadOfAnotherModule_keepsTheOldJarAndReportsFailure) and an overlapping update / failed later transfer (overlappingUpdateOfTheSameModule_isRefusedAndNoJarIsTruncated, laterUpdateWithFailedTransfer_leavesTheInstalledJarIntact) — 2/2 red each.

• /upm update reports success when the old module jar could not be deleted (updatePlugin ignores File#delete) #505 update replies (056faa4a): 8/8 command outcome tests red.

• Review round 4 (each fix reverted at its own commit): non-http link (134910d6, 1/1); same-file-system refusal and staging label (7bee9f8e, 3/3); unrestored original path and failure cause (760eb902, 4/4); raw plugin.yml version (73762181, 1/1); newer jar refusal (8a0dd765, 1/1); shared update/uninstall guard (4ae0b87c, 2/2); boot recovery (1b069df5, 4/6, the two others assert untouched states); command replies (0a1c5efb, 10/10); updatePlugin throws on an unrestored jar (86487c3a, 1/1).

• Review round 5 (7869a9eb, 3e9b4964, a76ac733, 00506f2a reverted together): 12/12 red — the eight journal-based recovery tests, the journal lifetime and uninstall-window tests, and the two reply tests.

Six mutations pin the lines a revert cannot reach, each killing exactly the assertion written for it: restoring a set-aside jar that no journal names (3 tests), keeping the journal after a successful transaction (2), recovering this process's own journal (1), removing the per-journal isolation of the boot sweep (1), never releasing the uninstall's keys (3), and taking those keys one at a time instead of all-or-none (1). The last two were the releases review round 4 measured as unpinned.

The staged transaction's rollback and ordering paths are covered by PluginInstallUtilsUpdateTransactionTest through a package-private file-operations seam, including a test that old jars are selected before the new version exists in the modules folder (the case-alias defect is excluded by construction, as it cannot be reproduced on a case-sensitive CI filesystem). Mutation runs confirm the tests catch removing the rollback move-back, skipping validation, and skipping the per-module guard, as well as reverting the uninstall call site to unregisterSelf() and dropping the jar outcome from an unload failure. Six further mutations found unpinned in review round 4 are each now caught by a test: the no-replace check before an atomic move, the guard key normalisation, the guard release in finally, the tolerance of a jar gone since selection, the loader's size/entry limit in validation, and deleting a partial download.

Checklist

• Targets alpha, unless this is an alpha -> main release promotion
• Line endings preserved per file (file <path> before and after; this tree is mixed CRLF/LF)
• Every comment, javadoc, workflow comment, and this PR's own title and body are English-first with Chinese as a supplement, and nothing was added to .github/cjk-allowlist.txt
• Documentation synced in UltiTools-Dev-Doc (branch alpha) if documented behaviour changed. Not owed; see the measurement above.
• FEATURES.md and UAT-CHECKLIST.md updated for every feature change in this PR:
  • amended ultitools.upm.uninstall, .neg-command-gone, .neg-scheduled-task-gone (now watching UltiCleaner's 15-second task), ultitools.upm.update and update-all;
  • added ultitools.upm.uninstall.neg-delete-failed, .neg-no-jar, ultitools.upm.update.neg-folder-read-only, ultitools.upm.update-all.neg-folder-read-only, ultitools.upm.update.neg-interrupted-recovered-on-boot and ultitools.upm.update.neg-leftover-without-journal-kept;
  • made two /upm update preconditions performable: the staging directory is cleared by one ordered procedure instead of a loop that never terminates, and "both folders are on the same file system" is now measured with findmnt and recorded blocked when it does not hold.

🤖 Generated with Claude Code

https://claude.ai/code/session_017BD9mGr6EsyRsEv3d2BizL

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-20T13:24:18.512758Z	a3e4a41	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[wisdommen]

@codex review

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 552 complexity

Metric	Results
Complexity	552

View in Codacy

🟢 Coverage 88.70% diff coverage · +0.76% coverage variation

Metric	Results
Coverage variation	✅ +0.76% coverage variation (-1.00%)
Diff coverage	✅ 88.70% diff coverage

View coverage diff in Codacy

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (5095455)	14419	11669	80.93%
Head commit (46252a8)	15147 (+728)	12374 (+705)	81.69% (+0.76%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#508)	770	683	88.70%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: fbf75729c8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 039c8f2e03

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Persist the confirmation phase before exposing the candidate

Fresh evidence beyond the handled journal-write failure is that abrupt termination bypasses this rollback branch: if the JVM stops after installNewVersion succeeds but before this call records the phase, the next boot sees an unmarked journal and an installed target, treats the set-aside JARs as leftovers, and deletes the journal in recoverJournal (lines 1552-1560). If the candidate then fails to load, the confirmation hook has no journal from which to restore the working version, defeating the transaction's crash-safety guarantee; record a recoverable phase before installing the target or preserve this installed-but-unmarked state for boot confirmation.

Useful? React with 👍 / 👎.

[wisdommen]

Correct, and fixed in f021b4f9 as row A8 of the crash-window table now in the pull request body.

Rather than keep the phase and the install in one order, the state is read from what is on disk: a journal of a dead process whose recorded target is installed as a JAR of that module is the confirmation hook's business whether or not the marker was written. Pre-load recovery hands it over, confirmation accepts it, and the branch that used to call those set-aside JARs leftovers of a finished update no longer exists — an installed candidate is never "finished" until a boot has seen whether the module loads.

Pinned by ModuleUpdateRecoveryTest#installedButUnmarkedJournal_isLeftForBootConfirmation; reverting it fails that test (UltiTools-Reborn-505-MUTATION-sweep-a8-RED.log).

[chatgpt-codex-connector]

Replace the existing journal portably

markJournalAwaitingBoot invokes this method after the journal already exists, so this atomic move must replace an existing target. With ATOMIC_MOVE, Files.move explicitly leaves existing-target behavior implementation-specific and permits providers to throw instead of replacing it; on such a filesystem every otherwise successful update reaches the marker-failure rollback path and can never become awaiting boot confirmation. Use a journal-replacement procedure that is guaranteed by the supported filesystem/provider contract.

Useful? React with 👍 / 👎.

[wisdommen]

Agreed, and fixed in f021b4f9 as row A18.

UpdateFileOperations now carries replace(source, target): it tries one atomic replace and, on AtomicMoveNotSupportedException, FileAlreadyExistsException or UnsupportedOperationException, removes the target and retries the rename atomically. A crash in that window leaves an installed candidate with no journal marker, which is row A8 above and is now read correctly — so the fallback's non-atomicity costs nothing.

Pinned by PluginInstallUtilsUpdateTransactionTest#journalRewriteOnAProviderThatRefusesAnAtomicReplace_stillMarksTheUpdate, which drives the update through a provider that refuses the replace; reverting the fallback fails it (…-MUTATION-sweep-replace-RED.log). A comment would not have caught it.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 039c8f2e03

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Refuse updates already awaiting boot confirmation

After an update returns UPDATED, this in-memory guard is released even though its journal remains pending, and the cached /upm update entry allows the same command to be run again before restart. The second transaction moves the first candidate aside and creates another awaiting journal; if the candidate then fails to load, confirmUpdatesAfterBoot processes those UUID-named journals in arbitrary lexical order and can restore the rejected candidate, delete both journals, and leave boot choosing between it and the original JAR. Treat an awaiting journal for this identify-string as ALREADY_IN_PROGRESS until boot confirms or rolls it back.

Useful? React with 👍 / 👎.

[wisdommen]

Real, and fixed in f021b4f9 as row B1 of the "what is legal while a journal is unresolved" table.

runUpdateTransaction now refuses before it downloads anything when a journal in the staging directory names this module in a boot-confirmation phase — ALREADY_IN_PROGRESS, nothing changed. The in-memory guard still covers concurrent transactions; this is the disk answering for the module in the window between the transaction and the restart, which the in-memory guard cannot span. The reply says which of the two it is, in both language files, and UAT-CHECKLIST.md carries the row that produces it.

Pinned by PluginInstallUtilsUpdateTransactionTest#secondUpdateWhileOneAwaitsTheRestart_isRefused (…-MUTATION-sweep-b1-RED.log).

[chatgpt-codex-connector]

Finalize failed updates that have no previous JAR

When an update has no older matching JAR—such as a loaded module whose source JAR was removed, or a direct first-time call—and its candidate fails to load, deleting installed fully completes the rollback because pairs is empty. Nevertheless rolledBack remains false here, so the journal is retained and every subsequent boot repeats the same severe failure indefinitely. Delete the journal and report the module as uninstalled when there were no rollback pairs, while retaining it only when a recorded pair still needs restoration.

Useful? React with 👍 / 👎.

[wisdommen]

Correct, and fixed in f021b4f9 as row A15.

The rollback now separates "everything this journal recorded has been dealt with" from "a previous version is installed again". When every pair has settled the journal goes, even if nothing came back; the operator is then told the module had no previous version to put back and is not installed, rather than reading the same SEVERE at every start with nothing left to do. A pair that genuinely still needs restoring keeps the journal, which is row A13's case.

Pinned by ModuleUpdateRecoveryTest#rollbackWithNoPreviousVersion_deletesItsJournal (…-MUTATION-sweep-nopairs-RED.log).

[chatgpt-codex-connector]

Preserve incomplete journals during boot confirmation

For an awaiting journal with a missing or unusable pair before a later aside.N entry, journalPairs signals the incomplete read through the supplied AtomicBoolean, but this call discards that signal. Confirmation can therefore process only the prefix and delete the journal—unconditionally on successful load, or after any prefix restore on failed load—stranding the later set-aside JAR with no recovery record. Inspect the skipped-pair flag and retain the journal whenever the pair list was incomplete.

Useful? React with 👍 / 👎.

[wisdommen]

Correct, and fixed in f021b4f9 as row C1 of the discarded-signals table — which was built by grepping the new paths for returned statuses rather than by recall, and this was the one being dropped.

confirmOneUpdate now keeps journalPairs' flag and passes it to both branches: the confirm path keeps the journal and says which JAR it could not read, and the rollback path treats the reading as unsettled, so the journal survives for a later start either way.

Pinned by ModuleUpdateRecoveryTest#confirmationOfAJournalWithAGap_keepsIt (…-MUTATION-sweep-c1-RED.log). The same flag is still discarded in leaveForBootConfirmation, deliberately and recorded as row C2: a pair it did not read is not marked accounted for, so the leftover pass names that JAR as belonging to a kept journal, which is the report that case needs.

[wisdommen]

@codex review Head is 58666dec. Since your last review the prediction redesign is complete and all five open findings are fixed, each test-first with a mutation log:

• r22 P1 (pre-load hook consumed awaiting journals) — recoverJournal returns on phase=awaiting-boot-confirmation, leaving the journal and its asides for confirmUpdatesAfterBoot.
• r22 P2 (marker write fails after the move-in) — the update undoes itself while both versions are on disk and reports NEW_JAR_NOT_INSTALLED.
• r23 P1 (confirm by shared name) — confirmation is keyed on the identify-string the journal records; UltiTools passes getIdentifyString().
• r23 P2 (partial rollback deleted its journal) — every pair must settle before the journal goes; otherwise it is kept for the next start and said so at SEVERE.
• r23 P2 (unreadable JAR beside a matching one) — reported, narrowed by file name so r6's unrelated zz-corrupt.jar still does not stop an uninstall. The reasoning is on that thread.

mvn -B clean verify and the isolated group are green (5880 + 23), CJK gate 0 violations.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58666dec27

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Mark retained partial rollbacks as rollback work

Fresh evidence after the r23 partial-rollback fix is that this retained journal still has the awaiting-boot-confirmation phase. On the next start, pre-load recovery deliberately skips it; if any old JAR was restored successfully, that JAR loads with the same identify-string, so confirmOneUpdate treats the rollback as a successful update and deletes every still-unrestored aside plus the journal. Change the phase or otherwise ensure the next boot finishes rollback rather than confirming it.

Useful? React with 👍 / 👎.

[wisdommen]

Correct, and fixed in f021b4f9 as row A13.

The journal now carries a third phase, rolling-back, written before the rollback touches anything. A start that finds it finishes the rollback regardless of what loaded — which is what makes a restored old JAR stop being mistaken for proof that the update loaded. Pre-load recovery leaves that phase alone exactly as it leaves awaiting-boot-confirmation.

Pinned by ModuleUpdateRecoveryTest#interruptedRollback_isFinishedAtTheNextBoot: two old JARs, one path occupied, then a second boot where the restored JAR does load. Reverting the phase write fails it (…-MUTATION-sweep-a13-RED.log). A failure to write that phase is row C4: logged at SEVERE, and the rollback still goes ahead, because leaving a module that cannot load installed is the worse outcome.

[chatgpt-codex-connector]

Report suspect JARs when no instance was loaded

Fresh evidence beyond the r23 unreadable-second-copy test is that reporting is still conditional on moduleUnloaded. When a module failed to load this boot but /upm uninstall finds and deletes one readable matching JAR, a same-prefixed unreadable second copy leaves suspects nonempty while this condition is false; the command returns success, and that copy can load the module after it becomes readable. The suspect should be reported whenever a matching uninstall otherwise succeeds, not only when a live instance was unloaded.

Useful? React with 👍 / 👎.

[wisdommen]

Ruled on by the maintainer, and fixed in f021b4f9: a suspect JAR is reported whether or not a live instance was unloaded. You are right that a module can fail to load and therefore have no instance to unload, while its second copy is no less able to load it later.

This does not reopen r6's "an unrelated unreadable JAR must not stop an uninstall", because reporting is not refusing: what counts as a suspect is still narrowed by the only evidence left once the metadata cannot be read — the file name, matched against the module's name and the identify-strings of the instances being unloaded. Both conditions hold at once.

Pinned by PluginInstallUtilsUninstallTest#uninstall_reportsASuspectJarEvenWhenNothingWasUnloaded; restoring the moduleUnloaded condition fails it and the r23 test with it (…-MUTATION-sweep-suspects-RED.log), while r6's unreadableEntriesInTheModulesFolder_areSkipped stays green.

[chatgpt-codex-connector]

Remove the candidate before restoring its target path

Fresh evidence after the r22 marker-failure fix is the case where an existing module JAR already has the candidate's final filename, such as a same-version retry: that old JAR is among movedAside, but moveBack runs while the candidate still occupies its original target. The restore either fails because the destination exists, or atomically replaces the candidate; the following delete then removes whichever file is at the target, and the journal is deleted even if the old JAR remains stranded. Delete the candidate first, then restore, and retain the journal if any restore fails.

Useful? React with 👍 / 👎.

[wisdommen]

Correct, and fixed in f021b4f9 as row A16.

The candidate is deleted first, and the restore then runs through the same rollBack every other failure of this transaction uses — so the journal is kept when a JAR stays in staging instead of being deleted unconditionally. The shorter open-coded version this branch had is gone; there is one rollback path.

Pinned by PluginInstallUtilsUpdateTransactionTest#markerFailureRollback_whenTheOldJarCarriesTheCandidateName, which is the same-version retry you describe: the set-aside JAR's path is the candidate's path. Restoring first fails it (…-MUTATION-sweep-marker-order-RED.log).

[wisdommen]

@codex review Head is 7161ece1. The eight findings of your last three reviews are fixed, and rather than fix them one at a time the durable state was enumerated in one pass: three tables are now in the pull request description — every gap between two consecutive durable writes (A1-A20), every operation an operator can run while a journal is unresolved (B1-B8), and every status the new code receives and drops (C1-C9). Eight rows were wrong; each fix is pinned by a test that fails when the fix is reverted in a throwaway copy.

Your findings map onto them: A8 (installed, journal unmarked), A18 (atomic replace refused by the provider), B1 (second update while one awaits a restart), A15 (rollback with nothing to put back), C1 (the dropped incomplete-read flag), A13 (interrupted rollback read as an update to confirm), A16 (candidate still holding the target path during the restore), plus the maintainer's ruling that a suspect JAR is reported whether or not a live instance was unloaded. Each thread carries its row and its commit.

Please check the tables rather than rediscover their rows: a finding that is a row is welcome, and a finding that names a row the tables do not have means the enumeration is incomplete, which is the more useful thing to tell me.

mvn -B clean verify 5888 tests green, isolated group 23 green, CJK gate 0 violations.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7161ece18a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Finish rolling-back journals before scanning JARs

When the process stops after writing phase=rolling-back but before deleting the rejected candidate, this pre-load recovery branch defers the journal to confirmUpdatesAfterBoot, which runs only after pluginManager.init. The rejected candidate is therefore included in the boot classpath and may load again; only afterward is it deleted and the old JAR restored, so the restored version cannot load until yet another restart. Handle rolling-back journals in pre-load recovery rather than grouping them with updates awaiting a verdict.

Useful? React with 👍 / 👎.

[wisdommen]

Correct, and fixed in 2c2c01d5. This is row A12, which the table had and whose verdict was wrong — recorded as such in the pull request description.

I had reasoned only about consistency (either phase is a defined state that the next start resolves) and missed the cost: the confirmation hook runs after pluginManager.init, so the rejected JAR is on the class path for that whole session and the JAR that does work goes back only afterwards. A rolling-back journal carries a verdict made at an earlier start and needs nothing from the load phase, so pre-load recovery now finishes it — deletes the rejected candidate, puts the old JARs back, and deletes the journal — before the class path is built. The confirmation hook keeps its own rolling-back branch for a rollback that began after the modules had loaded, and both callers run one body so the two cannot drift.

Pinned by ModuleUpdateRecoveryTest#rollingBackJournal_isFinishedBeforeTheModulesLoad and by interruptedRollback_isFinishedAtTheNextBoot, which now runs the real boot order; reverting the pre-load branch fails both (UltiTools-Reborn-505-MUTATION-sweep-a12-RED.log).

[chatgpt-codex-connector]

Refuse updates when an existing journal cannot be read

If an awaiting journal is temporarily unreadable or malformed, this continue treats it as though no update owns the module. A later /upm update can then move the first candidate aside and create a second journal; once the original journal becomes readable, boot has the same two-verdict rollback ambiguity that the new B1 guard is intended to prevent. Since the module identity cannot be ruled out here, conservatively refuse the update while any journal cannot be inspected.

Useful? React with 👍 / 👎.

[wisdommen]

Agreed, and fixed in 2c2c01d5. This is row C7, which the table had and disposed of wrongly: I recorded the continue as accepted because such a journal is reported elsewhere, which answers a different question than the one this check asks.

The check asks whether anything in staging forbids starting an update, and an unreadable journal cannot answer it — so it is now refused, reported as an unusable staging state naming the file, before anything is downloaded. That matches what the uninstall path already does with an unreadable journal of the module being removed (r9): the same evidence, the same conservative answer.

Pinned by PluginInstallUtilsUpdateTransactionTest#updateWhileAJournalCannotBeRead_isRefused; restoring the continue fails it (UltiTools-Reborn-505-MUTATION-sweep-c7-RED.log).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4cb43c18f6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve restored JARs when retrying a rollback

When a rejected same-version retry has a recorded original path equal to target, the first rollback restores the working JAR to installed. If another pair or an incomplete journal keeps the transaction in rolling-back (or journal deletion fails), the next boot enters this method again and unconditionally deletes that restored working JAR; its aside is already gone, so the pair is then treated as settled and cannot be restored again. Make candidate removal idempotent or record that it has already occurred instead of deleting the target on every rollback retry.

Useful? React with 👍 / 👎.

[wisdommen]

Correct, and fixed in a3a80abb. The tables had no row for a retried rollback — the enumeration was incomplete, the row is now A21 in the pull request description, and this is where it came from.

The rollback deletes what sits at the rejected update's target path because that is the candidate — except after an attempt that already restored a set-aside JAR whose own path is that target, which is exactly the same-version retry. Its staging copy is then gone, so the next attempt would take the module's only remaining copy with nothing to restore it from. The pairs and the staging directory already say which case it is: a pair whose original equals the target and whose set-aside file no longer exists means the file there is restored, not rejected. No new state, and no marker to keep in step.

Pinned by ModuleUpdateRecoveryTest#rollbackRetried_keepsWhatTheFirstAttemptRestored — one restored same-name pair plus a second pair whose path is occupied, so the journal survives into a second attempt. Removing the check fails it (UltiTools-Reborn-505-MUTATION-sweep-a21-RED.log).

[chatgpt-codex-connector]

Keep the old journal until its replacement is published

Fresh evidence after the earlier portable-replacement fix is that this fallback deletes the only existing journal before publishing its replacement. During markJournalAwaitingBoot, the candidate is already installed and the previous JARs are already in staging, so a process termination between these two calls—or a failure of the second atomic move—leaves no journal at all; the next boot consequently cannot roll back an unloadable candidate and treats the old JARs as unreferenced leftovers. Preserve the original journal until the replacement has been successfully published.

Useful? React with 👍 / 👎.

[wisdommen]

Correct, and fixed in a3a80abb. This is row A18, whose window I read wrongly: I wrote that a crash inside the fallback lands in row A8, and A8 needs a journal to exist, which is the one thing this window does not leave.

Rather than keep the old journal — one name, one slot, and no portable way to hold both — the recovery now knows what a .txn.tmp can be. Beside its journal it is a half-written file and is deleted as before. Alone, and complete, it is the replacement whose rename never finished: it is renamed into place and recovered, so the set-aside JARs keep their record and an unloadable candidate can still be rolled back. Alone and incomplete — a journal that was never finished being created — it is deleted, as before.

Pinned by ModuleUpdateRecoveryTest#completeTemporaryJournal_isAdoptedWhenTheJournalIsGone; deleting every temporary journal as stale fails it (UltiTools-Reborn-505-MUTATION-sweep-a18tmp-RED.log).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2c2c01d52c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Abort rollback when the candidate cannot be deleted

Fresh evidence after the earlier marker-write rollback fix is that deleteQuietly discards a failure to remove the installed candidate and rollback proceeds anyway. If the confirmation marker cannot be written and this deletion also fails, old JARs with different filenames can all be restored successfully, causing rollBack to delete the journal even though the candidate remains installed; the command reports failure, but the next restart sees both versions and may load the rejected candidate. Candidate deletion must be verified before restoring the old JARs, with the journal retained when it fails.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Require a durable rolling-back phase before mutating files

If writing phase=rolling-back fails before replacing the existing journal, this call leaves the journal marked awaiting-boot-confirmation but immediately starts mutating files. A crash or restore failure after one old JAR has returned then lets that old module load on the next boot; confirmation interprets it as a successful update and deletes the still-unrestored set-aside JARs. Do not begin rollback unless its phase is durably recorded, or retain another state that pre-load recovery will unambiguously finish.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a3a80abb81

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Coordinate direct installs with update transactions

When /upm update is downloading asynchronously, /upm install can install another version of the same identify-string because the install paths neither acquire MODULE_OPERATIONS_IN_PROGRESS nor check pending journals. Since this line snapshots the existing JARs before that download finishes, the concurrently installed JAR is absent from olderJars; the update can then return UPDATED while leaving both versions in the modules folder. Route direct installs through the same module guard and reject them while an update is running or awaiting boot confirmation.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Retain unreadable temporary journals

When the replacement fallback was interrupted after deleting the old .txn, the .txn.tmp can be the only record protecting the set-aside JARs. If that temporary file is merely unreadable during the next boot, isCompleteJournal returns false and this branch deletes it as though it were proven half-written; the old JARs are then orphaned and an unloadable candidate cannot be rolled back. Distinguish an unreadable temporary journal from a successfully parsed but incomplete one and retain the former.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a3e4a41128

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Confirm the updated artifact rather than any matching identity

Fresh evidence beyond the earlier shared-runtime-name thread is that findPluginJars skips an old JAR while it is unreadable, allowing the update to leave that copy in the modules folder. If it becomes readable at boot and the candidate itself fails to load, the old copy supplies the same identify-string, so this condition confirms the failed candidate, deletes the rollback material and journal, and leaves the rejected candidate installed. Confirmation needs to identify the loaded target/version rather than accepting any loaded version of the module.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Surface unsupported atomic moves through the boolean API

When an actual move throws AtomicMoveNotSupportedException and rollback restores everything, the transaction returns ATOMIC_MOVE_UNSUPPORTED with an empty unrestored list, but this condition excludes that status and updatePlugin silently returns false. That contradicts this public method's new contract that failed JAR moves throw an UncheckedIOException, and prevents legacy callers from receiving the filesystem-specific failure needed to diagnose why updates cannot work on that provider.

Useful? React with 👍 / 👎.

[wisdommen]

Closing in favour of #561.

On 2026-09-29 the maintainer decided to finish #505 by observation rather than prediction: instead of deciding before a restart whether a downloaded JAR will load, /upm update now stages the new JAR, the next start swaps the files before any module class loads, and the update is kept only when that start shows the module actually loaded from the new JAR at the new version (otherwise the old JAR is restored). This pull request's prediction route — 29 review rounds, 13 of whose findings named the previous round's fix as their cause — is not continued. No code from this branch was carried over; it was read only for its list of crash windows. The branch fix/503-upm-uninstall-single-unload-path is kept for its history. #501 and #503 were already closed.

---

改由 #561 接替，本 PR 关闭。

维护者 2026-09-29 决定：#505 改用观察而不是预测来完成——不再在重启前判断下载的 JAR 能不能加载，而是 /upm update 只暂存新 JAR，下次启动在加载任何模块类之前换文件，只有这次启动确实从新 JAR 以新版本加载了该模块，更新才保留，否则恢复旧 JAR。本 PR 的预测路线（29 轮评审，其中 13 条发现指向上一轮的修复）不再继续。本分支的代码没有被搬运，只参考了它列出的崩溃窗口。分支 fix/503-upm-uninstall-single-unload-path 保留作历史记录。#501 和 #503 此前已关闭。