Added Registration migration process document#763
Open
ghost wants to merge 3 commits intomosip:developfrom
Open
Added Registration migration process document#763ghost wants to merge 3 commits intomosip:developfrom
ghost wants to merge 3 commits intomosip:developfrom
Conversation
WalkthroughAdds two new docs: a design for a two‑phase, launcher-driven Registration Client upgrade from Java 11 → 21 using staged artifacts, dual MANIFESTs, resumable native migration/rollback executables; and an agent guide ( Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant Operator as Operator
end
rect rgba(200,255,200,0.5)
participant Launcher as _launcher.jar
participant App as registration-client.jar
participant JRE as JRE (11 / 21)
participant FS as FileSystem (.artifacts/, lib/, .TEMP/)
participant Migrator as migration.exe / rollback.exe
end
Operator->>Launcher: Start process
Launcher->>JRE: Read java.version
alt Running on Java 11
Launcher->>FS: Verify ./MANIFEST.MF.sig and read root MANIFEST
FS-->>Launcher: Provide staged artifacts (.artifacts/)
Launcher->>Operator: Request restart to run migrator
Operator->>Launcher: Restart -> Launcher invokes Migrator
Launcher->>Migrator: Start migration (backup, promote JRE, extract lib.zip, restore run.bat)
Migrator->>FS: Modify jre/, lib/, cleanup .TEMP/
Migrator-->>Operator: Signal completion
else Running on Java 21+
Launcher->>FS: Validate lib/MANIFEST.MF and per-file hashes
alt lib upgrade present
FS-->>Launcher: lib.zip in .TEMP/
Launcher->>FS: Unzip/resume if needed
end
Launcher->>App: Launch normal application flow
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@design/registration/registration-upgrade.md`:
- Line 23: The heading "Upgrade MANIFEST.MF contains the following entries:"
uses four hashes (####) and skips a level; update that heading to the correct
level (e.g., change "#### Upgrade MANIFEST.MF contains the following entries:"
to "### Upgrade MANIFEST.MF contains the following entries:") so the document
outline and markdownlint rules are satisfied.
- Around line 111-115: The fixed "timeout /t 5" is racy because it assumes the
old Java process released jre; replace it with a loop that checks for
process/handle release before moving: detect running Java processes (e.g., via
tasklist filtering for java.exe or by testing whether the jre folder is locked)
and wait/retry until no process holds files or a configurable max-wait is
reached, then perform the backup moves (the lines with "timeout /t 5", "move jre
jre_old", and "move jre21_temp jre"). Ensure the loop logs retries and exits
with an error if the timeout is exceeded so the upgrade does not leave a partial
state.
- Around line 126-134: Define and document a concrete manifest entry and
signature format and update the verifier/creator accordingly: specify that each
MANIFEST.MF entry produced by ManifestCreator will include filename,
hash-algorithm, digest (base64/hex), optional size, and that the file-level
digests are not individually signed but the entire manifest is signed; define a
canonical manifest serialization (sorted entries, normalized line endings) and a
manifest-level signature block containing signer id, signature-algorithm,
signature (base64), and timestamp. Change ManifestCreator to emit the canonical
manifest and append the manifest signature generated with the private key.
Change SoftwareUpdateUtil to first verify the manifest signature against the
embedded signer id using the client’s public key store, then parse entries and
validate each file digest as before; on signature verification failure reject
the manifest immediately. Also document signature algorithm choices and where
the client public keys are stored and how key rotation/fallback is handled.
- Around line 94-96: The rollback described is incomplete: update the docs and
sample flow so rollback.bat and migration.bat fully preserve and restore the
entire pre-upgrade state (not just the jre). Specifically, change the flow
around migration.bat/migrator.bat to first copy the entire current installation
into a timestamped backup folder (including lib/*.jar, run.bat, config files and
any custom scripts), stage new files into temp locations (e.g., jre21_temp,
lib.TEMP, run.TEMP) without deleting originals, and then on success delete
backups; implement rollback.bat to restore from that backup folder by copying
back jre, all jars in lib, run.bat, configs, and cleaning temp folders; update
the documentation sections referencing rollback.bat, migration.bat,
migrator.bat, jre/jre21_temp, lib, and run.bat to describe these concrete
backup-and-restore steps and verification checks.
- Around line 109-124: The fenced code block containing the batch script
(starting with "@echo off" and "timeout /t 5 /nobreak > nul") needs a language
label to enable syntax highlighting and satisfy markdownlint; update the
triple-backtick fence to include a batch label such as ```bat (or ```batch)
above that block so editors and linters treat it as a Windows batch file.
- Around line 34-35: The doc's migration plan relies on duplicate
fully-qualified class Initialization existing in both launcher.jar and
registration-client.jar and on wildcard classpath expansion to determine which
one loads, which is non-deterministic; instead, remove duplicate Initialization
classes and add an explicit bootstrap entry point (e.g., RegistrationBootstrap
or LauncherInitialization) that clearly delegates to the chosen implementation,
update startup scripts/manifest to set the main class to that bootstrap, and
change classpath construction to list required JARs explicitly (no lib/*
wildcard) or use a URLClassLoader that programmatically loads the intended
registration-client.jar to guarantee deterministic loading and avoid relying on
JVM wildcard resolution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f8ef5cac-154e-4f34-85ad-4749ee33df43
📒 Files selected for processing (1)
design/registration/registration-upgrade.md
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
design/registration/registration-upgrade.md (6)
109-109:⚠️ Potential issue | 🟡 MinorAdd a language label to the batch script block.
The fenced block starting at Line 109 is still unlabeled, so editors and markdownlint cannot treat it as a batch script.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` at line 109, The fenced code block starting at the unlabeled fence (around the block beginning at Line 109) needs a language label so editors and markdownlint treat it as batch script; update the opening fence from ``` to ```bat (or ```batch) to mark it as a Windows batch script, ensuring the block is recognized and linted correctly.
23-23:⚠️ Potential issue | 🟡 MinorUse the next heading level here.
Line 23 still skips from
##to####, which breaks the outline and keeps markdownlint unhappy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` at line 23, The heading "#### Upgrade MANIFEST.MF contains the following entries:" uses level 4 while the previous heading is level 2; change the leading hashes from '####' to '###' so the outline progresses from '##' to '###' (i.e., update the heading "Upgrade MANIFEST.MF contains the following entries:" to use '###') to satisfy markdownlint and maintain a proper hierarchy.
94-96:⚠️ Potential issue | 🟠 MajorDocument a complete rollback path for full upgrades.
The rollback story is still incomplete. The sample flow preserves
jre, but it does not preserve or restore the oldlibcontents,run.bat, or other root-level files the design says may change.Also applies to: 107-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` around lines 94 - 96, Update the rollback documentation to describe a complete, reproducible rollback path for full upgrades: when generating rollback.bat and the backup folder (referenced alongside migrator.bat and migration.bat), explicitly state that the backup must include the entire original root (jre, lib, run.bat, any modified root-level files) and describe the restore steps to revert each (how to restore jre from jre backup, how to replace lib with its backed-up contents, how to restore run.bat and other root files, and how to handle file locks/process termination). Also add failure/verification checks the rollback should perform (e.g., checksum or existence checks) and update the narrative around migration.bat/migrator.bat to show creation of these full backups before any overwrite so rollback.bat can fully restore the previous state.
111-115:⚠️ Potential issue | 🟠 MajorReplace the fixed delay with an actual readiness check.
A hard-coded 5-second sleep is still too brittle for the JRE swap. The migration should wait until the old process has actually released the files or fail cleanly after a bounded timeout.
On Windows, can a batch script reliably rename or move a directory like `jre` while a Java process is still shutting down and may still hold files open?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` around lines 111 - 115, Replace the fixed sleep ("timeout /t 5 /nobreak > nul") with a bounded readiness loop that verifies the old Java process has released files before attempting the renames; implement a retry loop that attempts to move/rename "jre" (the "move jre jre_old" operation) and on failure waits a short interval and retries until a configured timeout, and/or explicitly check for running Java processes via tasklist/findstr and fail with an error message if the timeout is exceeded so the script exits cleanly instead of proceeding while files are still locked.
126-134:⚠️ Potential issue | 🟠 MajorSpecify the manifest signature format and verifier behavior.
This still says the manifests will contain “signed hashes,” but it does not define the actual manifest entry format, canonicalization rules, signature scope, or verifier flow. The integrity model is not implementable as written.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` around lines 126 - 134, Update the registration-upgrade spec to define a concrete manifest signature format and verifier flow: specify the manifest entry format (e.g., each line as "path:length:hex_sha256" or JSON objects), canonicalization rules (UTF-8, LF line endings, sorted by path, deterministic field order), signature scope (what bytes are signed — entire manifest file after canonicalization and optional metadata fields like version/timestamp), signature algorithm (e.g., RSA-PSS/SHA256 or ECDSA/secp256r1 with explicit key id), and verifier behavior (verify signature against the bundled public key, check manifest timestamp/version/revocation, verify each file hash matches its manifest entry, and fail/rollback on any mismatch). Reference both MANIFEST.MF in lib and application root and state that both use the same canonicalization and signature verification steps for patch and full upgrade code paths.
34-35:⚠️ Potential issue | 🟠 MajorDo not make migration depend on duplicate
Initializationclasses.This still relies on classpath ordering to choose between two classes with the same FQCN. That is not a stable bootstrap mechanism for an upgrade path; the design should use an explicit entry point instead.
Does Java guarantee which class is loaded when two JARs on the classpath contain the same fully-qualified class, and is wildcard expansion order for `lib/*` deterministic across JVMs/platforms?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` around lines 34 - 35, The design currently relies on two classes named Initialization (same FQCN) in launcher.jar and registration-client.jar which depends on JVM classpath ordering — change this to use an explicit, unique entry point instead: create a single uniquely named entry class (e.g., RegistrationMigrationEntrypoint or MigrationInitializer) or expose the migration implementation via a stable discovery mechanism (ServiceLoader SPI or an explicitly referenced class name in configuration/manifest) and update the launcher to invoke that explicit entry point (by direct reference or safe reflection lookup) rather than relying on the duplicate Initialization class; ensure you remove or rename the duplicate Initialization in registration-client.jar or move it to a different package so no two JARs contain the same FQCN.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@design/registration/registration-upgrade.md`:
- Around line 2-3: The document currently contradicts itself by claiming run.bat
is immutable while later describing steps that rewrite or recreate run.bat;
update the text to explicitly state the immutability scope (e.g., "the
pre-migration Java 11 client's run.bat is immutable and must not be modified")
and then describe the migration approach accordingly—either perform migration by
creating a new wrapper (e.g., run_migrated.bat or a launcher that delegates to
JRE21) or note that immutability is lifted for post-migration artifacts. Search
for every place run.bat and "Zero-Touch" are referenced (including the sections
describing the proposed flow) and make the wording consistent: prohibit direct
edits to the original run.bat where required and document the alternative (new
wrapper or atomic swap) that the migration will use.
- Around line 20-21: Replace the misspelled Maven metadata filename
'maven_metatdata.xml' with the correct 'maven-metadata.xml' wherever it appears
in the document (the phrase in the sentence about clients noticing the version
change and triggering the update should be updated accordingly), ensuring the
hyphenated spelling "maven-metadata.xml" is used consistently in the text.
---
Duplicate comments:
In `@design/registration/registration-upgrade.md`:
- Line 109: The fenced code block starting at the unlabeled fence (around the
block beginning at Line 109) needs a language label so editors and markdownlint
treat it as batch script; update the opening fence from ``` to ```bat (or
```batch) to mark it as a Windows batch script, ensuring the block is recognized
and linted correctly.
- Line 23: The heading "#### Upgrade MANIFEST.MF contains the following
entries:" uses level 4 while the previous heading is level 2; change the leading
hashes from '####' to '###' so the outline progresses from '##' to '###' (i.e.,
update the heading "Upgrade MANIFEST.MF contains the following entries:" to use
'###') to satisfy markdownlint and maintain a proper hierarchy.
- Around line 94-96: Update the rollback documentation to describe a complete,
reproducible rollback path for full upgrades: when generating rollback.bat and
the backup folder (referenced alongside migrator.bat and migration.bat),
explicitly state that the backup must include the entire original root (jre,
lib, run.bat, any modified root-level files) and describe the restore steps to
revert each (how to restore jre from jre backup, how to replace lib with its
backed-up contents, how to restore run.bat and other root files, and how to
handle file locks/process termination). Also add failure/verification checks the
rollback should perform (e.g., checksum or existence checks) and update the
narrative around migration.bat/migrator.bat to show creation of these full
backups before any overwrite so rollback.bat can fully restore the previous
state.
- Around line 111-115: Replace the fixed sleep ("timeout /t 5 /nobreak > nul")
with a bounded readiness loop that verifies the old Java process has released
files before attempting the renames; implement a retry loop that attempts to
move/rename "jre" (the "move jre jre_old" operation) and on failure waits a
short interval and retries until a configured timeout, and/or explicitly check
for running Java processes via tasklist/findstr and fail with an error message
if the timeout is exceeded so the script exits cleanly instead of proceeding
while files are still locked.
- Around line 126-134: Update the registration-upgrade spec to define a concrete
manifest signature format and verifier flow: specify the manifest entry format
(e.g., each line as "path:length:hex_sha256" or JSON objects), canonicalization
rules (UTF-8, LF line endings, sorted by path, deterministic field order),
signature scope (what bytes are signed — entire manifest file after
canonicalization and optional metadata fields like version/timestamp), signature
algorithm (e.g., RSA-PSS/SHA256 or ECDSA/secp256r1 with explicit key id), and
verifier behavior (verify signature against the bundled public key, check
manifest timestamp/version/revocation, verify each file hash matches its
manifest entry, and fail/rollback on any mismatch). Reference both MANIFEST.MF
in lib and application root and state that both use the same canonicalization
and signature verification steps for patch and full upgrade code paths.
- Around line 34-35: The design currently relies on two classes named
Initialization (same FQCN) in launcher.jar and registration-client.jar which
depends on JVM classpath ordering — change this to use an explicit, unique entry
point instead: create a single uniquely named entry class (e.g.,
RegistrationMigrationEntrypoint or MigrationInitializer) or expose the migration
implementation via a stable discovery mechanism (ServiceLoader SPI or an
explicitly referenced class name in configuration/manifest) and update the
launcher to invoke that explicit entry point (by direct reference or safe
reflection lookup) rather than relying on the duplicate Initialization class;
ensure you remove or rename the duplicate Initialization in
registration-client.jar or move it to a different package so no two JARs contain
the same FQCN.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ba32d09-a824-4249-b4ea-108e06681fc1
📒 Files selected for processing (1)
design/registration/registration-upgrade.md
Signed-off-by: anushasunkada <anushasunkada@gmail.com>
Signed-off-by: anushasunkada <anushasunkada@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
design/registration/registration-upgrade.md (3)
7-7:⚠️ Potential issue | 🟠 MajorResolve the
run.batimmutability contradiction (Line 7 vs Lines 96–97).The document says
run.batcannot be modified, but the migration flow overwrites rootrun.bat. Define immutability scope explicitly (pre-migration only vs always) and align all steps.Also applies to: 96-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` at line 7, Clarify and resolve the contradiction about run.bat by explicitly defining the immutability scope (e.g., "run.bat is immutable prior to migration" vs "run.bat may be overwritten during migration") and then update the migration flow steps that reference overwriting the root run.bat (the steps described around lines 96–97) to match that scope; specifically, either mark the migration step as an exception that modifies run.bat or change the migration instructions to preserve the original run.bat and create a new launcher, and make the same scope statement where run.bat is first introduced (the sentence referencing "Immutable Launcher" / run.bat) so all mentions are consistent.
21-21:⚠️ Potential issue | 🟡 MinorUse the correct Maven metadata filename at Line 21.
maven_metadata.xmlis incorrect; usemaven-metadata.xmlconsistently to avoid implementation drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` at line 21, Replace the incorrect filename "maven_metadata.xml" with the correct "maven-metadata.xml" in the registration-upgrade.md text (specifically the line that currently reads 'maven_metadata.xml' in the step "Client detects version change"), ensuring the hyphenated form is used consistently throughout the document.
68-68:⚠️ Potential issue | 🟠 MajorDo not rely on
lib/*ordering for bootstrap class selection at Line 68.Depending on wildcard classpath/alphabetical ordering for
_launcher.jarprecedence is not a deterministic contract across environments. The entrypoint must be explicit.Does Java guarantee deterministic JAR ordering for wildcard classpaths like `lib/*`, and can that be safely relied on to choose which duplicate class is loaded first?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` at line 68, The doc is relying on wildcard classpath ordering to ensure _launcher.jar loads before registration-client.jar; instead make the entrypoint explicit by changing the bootstrap to invoke _launcher.jar directly (e.g., use java -jar _launcher.jar or an explicit -cp with _launcher.jar first) or ensure _launcher.jar defines a Main-Class and the launcher is executed by name, and remove/ensure any main() from registration-client.jar (the old Initialization class) is not on the classpath at bootstrap; update references to run.bat, _launcher.jar, registration-client.jar, and Initialization so the startup sequence no longer depends on lib/* ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@design/registration/registration-upgrade.md`:
- Line 263: Escape the underscores by wrapping `_launcher.jar` in backticks at
the sentence "ClientPreLoader processes .UNKNOWN_JARS entries before
_launcher.jar Initialization" and also wrap `.UNKNOWN_JARS` in backticks where
appropriate; update the phrase to use `ClientPreLoader processes `.UNKNOWN_JARS`
entries before ` _launcher.jar` Initialization` (preserve wording aside from
adding backticks) so Markdown lint MD037 is satisfied. Locate the sentence by
the symbol ClientPreLoader and adjust both `_launcher.jar` and `.UNKNOWN_JARS`
occurrences throughout the document.
- Around line 102-105: Add a blank line immediately before the table that starts
with "| Trigger | Invoked By | Condition |" and a blank line immediately after
the table (the section under the "**When it is invoked:**" heading) so the
Markdown table is separated by empty lines; update the block containing the
table under the "**When it is invoked:**" paragraph to ensure an empty line
above and below to satisfy MD058.
---
Duplicate comments:
In `@design/registration/registration-upgrade.md`:
- Line 7: Clarify and resolve the contradiction about run.bat by explicitly
defining the immutability scope (e.g., "run.bat is immutable prior to migration"
vs "run.bat may be overwritten during migration") and then update the migration
flow steps that reference overwriting the root run.bat (the steps described
around lines 96–97) to match that scope; specifically, either mark the migration
step as an exception that modifies run.bat or change the migration instructions
to preserve the original run.bat and create a new launcher, and make the same
scope statement where run.bat is first introduced (the sentence referencing
"Immutable Launcher" / run.bat) so all mentions are consistent.
- Line 21: Replace the incorrect filename "maven_metadata.xml" with the correct
"maven-metadata.xml" in the registration-upgrade.md text (specifically the line
that currently reads 'maven_metadata.xml' in the step "Client detects version
change"), ensuring the hyphenated form is used consistently throughout the
document.
- Line 68: The doc is relying on wildcard classpath ordering to ensure
_launcher.jar loads before registration-client.jar; instead make the entrypoint
explicit by changing the bootstrap to invoke _launcher.jar directly (e.g., use
java -jar _launcher.jar or an explicit -cp with _launcher.jar first) or ensure
_launcher.jar defines a Main-Class and the launcher is executed by name, and
remove/ensure any main() from registration-client.jar (the old Initialization
class) is not on the classpath at bootstrap; update references to run.bat,
_launcher.jar, registration-client.jar, and Initialization so the startup
sequence no longer depends on lib/* ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85c29c85-bb12-4f37-94f5-03c2a5c1d88c
📒 Files selected for processing (2)
AGENTS.mddesign/registration/registration-upgrade.md
✅ Files skipped from review due to trivial changes (1)
- AGENTS.md
Comment on lines
+102
to
+105
| **When it is invoked:** | ||
| | Trigger | Invoked By | Condition | | ||
| |---------|-----------|-----------| | ||
| | `migration.exe` fails mid-execution | `migration.exe` itself | `jre11/` backup exists AND `jre/` is missing or partially populated | |
There was a problem hiding this comment.
Add required blank lines around the table near Line 103.
Markdownlint MD058 is triggered because the table is not surrounded by blank lines.
Suggested markdown fix
**When it is invoked:**
+
| Trigger | Invoked By | Condition |
|---------|-----------|-----------|
| `migration.exe` fails mid-execution | `migration.exe` itself | `jre11/` backup exists AND `jre/` is missing or partially populated |
| `jre21_temp/` unzip incomplete | `migration.exe` | `jre21_temp/` exists but JRE rename cannot complete |
| Disk space exhaustion during `migration.exe` | `migration.exe` | Any file operation throws an out-of-space exception |
| Inconsistent JRE state on startup | `_launcher.jar` (step 3) | `System.getProperty("java.version")` returns an unknown/unexpected version |
| Operator force-kills during `migration.exe` and JRE is unrecognisable on next restart | `_launcher.jar` (step 3) | JRE version is neither 11 nor 21 |
| Operator manually runs `rollback.exe` | Operator | Any time post-migration failure; remains available until step 6 cleanup |
+🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 103-103: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@design/registration/registration-upgrade.md` around lines 102 - 105, Add a
blank line immediately before the table that starts with "| Trigger | Invoked By
| Condition |" and a blank line immediately after the table (the section under
the "**When it is invoked:**" heading) so the Markdown table is separated by
empty lines; update the block containing the table under the "**When it is
invoked:**" paragraph to ensure an empty line above and below to satisfy MD058.
| **Context:** An old .UNKNOWN_JARS file lists jre21.zip as unknown | ||
|
|
||
| **Handling:** | ||
| - ClientPreLoader processes .UNKNOWN_JARS entries before _launcher.jar Initialization |
There was a problem hiding this comment.
Wrap _launcher.jar in code formatting at Line 263.
Unescaped underscore usage in prose is tripping markdownlint (MD037). Use backticks for _launcher.jar (and similarly for .UNKNOWN_JARS where applicable).
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 263-263: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@design/registration/registration-upgrade.md` at line 263, Escape the
underscores by wrapping `_launcher.jar` in backticks at the sentence
"ClientPreLoader processes .UNKNOWN_JARS entries before _launcher.jar
Initialization" and also wrap `.UNKNOWN_JARS` in backticks where appropriate;
update the phrase to use `ClientPreLoader processes `.UNKNOWN_JARS` entries
before ` _launcher.jar` Initialization` (preserve wording aside from adding
backticks) so Markdown lint MD037 is satisfied. Locate the sentence by the
symbol ClientPreLoader and adjust both `_launcher.jar` and `.UNKNOWN_JARS`
occurrences throughout the document.
Signed-off-by: anushasunkada <anushasunkada@gmail.com>
There was a problem hiding this comment.
♻️ Duplicate comments (4)
design/registration/registration-upgrade.md (4)
102-111:⚠️ Potential issue | 🟡 MinorAdd blank lines around the table block.
The table under “When it is invoked:” still violates MD058 without surrounding blank lines.
Suggested markdown fix
**When it is invoked:** + | Trigger | Invoked By | Condition | |---------|-----------|-----------| | `migration.exe` fails mid-execution | `migration.exe` itself | `jre11/` backup exists AND `jre/` is missing or partially populated | | `jre21_temp/` unzip incomplete | `migration.exe` | `jre21_temp/` exists but JRE rename cannot complete | | Disk space exhaustion during `migration.exe` | `migration.exe` | Any file operation throws an out-of-space exception | | Inconsistent JRE state on startup | `_launcher.jar` (step 3) | `System.getProperty("java.version")` returns an unknown/unexpected version | | Operator force-kills during `migration.exe` and JRE is unrecognisable on next restart | `_launcher.jar` (step 3) | JRE version is neither 11 nor 21 | | Operator manually runs `rollback.exe` | Operator | Any time post-migration failure; remains available until step 6 cleanup | +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` around lines 102 - 111, The Markdown table under the "When it is invoked:" heading violates MD058; fix it by adding one blank line immediately before the table and one blank line immediately after the table so the table is separated from surrounding paragraphs/headings; locate the table block that starts with the header row "| Trigger | Invoked By | Condition |" under the "When it is invoked:" section and insert the blank lines around it.
263-263:⚠️ Potential issue | 🟡 MinorWrap underscore-containing filenames in backticks.
.UNKNOWN_JARSand_launcher.jarin plain text trigger markdown emphasis parsing issues (MD037).Suggested markdown fix
-- ClientPreLoader processes .UNKNOWN_JARS entries before _launcher.jar Initialization +- ClientPreLoader processes `.UNKNOWN_JARS` entries before `_launcher.jar` Initialization🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` at line 263, The markdown line uses .UNKNOWN_JARS and _launcher.jar which trigger MD037; wrap both filenames in backticks (e.g., `.UNKNOWN_JARS` and `_launcher.jar`) in the registration-upgrade.md text so underscores are treated as code and not emphasis, ensuring the line reads "ClientPreLoader processes `.UNKNOWN_JARS` entries before `_launcher.jar` Initialization".
68-68:⚠️ Potential issue | 🟠 MajorDo not treat
-cp lib/*ordering as guaranteed.The statement that
_launcher.jaris guaranteed to load first via alphabetical ordering is unsafe; wildcard classpath expansion order is not a portable guarantee. This can break the<1.3.0 -> 1.3.0transition path nondeterministically.Suggested doc fix
-1. `run.bat` launches application, JVM loads `_launcher.jar` first (guaranteed by alphabetical sorting: `_launcher.jar` = ASCII 95 < 'a' = ASCII 97). This ordering guarantee is critical only for the one-time `<1.3.0 → 1.3.0` transition, where the old `registration-client.jar` still contains an `Initialization` class. From 1.3.0 onwards, `registration-client.jar` has no `main()` entry point, so `_launcher.jar` is unambiguously the sole entry point regardless of classpath order. +1. `run.bat` launches application. Do not rely on `-cp lib/*` wildcard expansion order for entrypoint selection during `<1.3.0 → 1.3.0`. Ensure a deterministic bootstrap mechanism (explicit main class/launcher indirection) so `_launcher.jar` is always selected intentionally.Does the Java launcher guarantee deterministic jar ordering for wildcard classpaths like `lib/*`, and can alphabetical jar naming be relied on for class loading priority?
21-21:⚠️ Potential issue | 🟡 MinorUse the standard Maven metadata filename.
maven_metadata.xmlis not the standard repository metadata file; usemaven-metadata.xmlto avoid implementation drift.What is the standard Maven repository metadata filename used by clients for version discovery?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@design/registration/registration-upgrade.md` at line 21, The doc and any client code refer to the non-standard filename "maven_metadata.xml"; update the reference to the correct Maven metadata filename "maven-metadata.xml" wherever the client polls repository metadata (e.g., the "Client detects version change" step that polls maven_metadata.xml) and update any code that constructs or requests that filename to use "maven-metadata.xml" instead so clients perform version discovery against the standard Maven metadata file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@design/registration/registration-upgrade.md`:
- Around line 102-111: The Markdown table under the "When it is invoked:"
heading violates MD058; fix it by adding one blank line immediately before the
table and one blank line immediately after the table so the table is separated
from surrounding paragraphs/headings; locate the table block that starts with
the header row "| Trigger | Invoked By | Condition |" under the "When it is
invoked:" section and insert the blank lines around it.
- Line 263: The markdown line uses .UNKNOWN_JARS and _launcher.jar which trigger
MD037; wrap both filenames in backticks (e.g., `.UNKNOWN_JARS` and
`_launcher.jar`) in the registration-upgrade.md text so underscores are treated
as code and not emphasis, ensuring the line reads "ClientPreLoader processes
`.UNKNOWN_JARS` entries before `_launcher.jar` Initialization".
- Line 21: The doc and any client code refer to the non-standard filename
"maven_metadata.xml"; update the reference to the correct Maven metadata
filename "maven-metadata.xml" wherever the client polls repository metadata
(e.g., the "Client detects version change" step that polls maven_metadata.xml)
and update any code that constructs or requests that filename to use
"maven-metadata.xml" instead so clients perform version discovery against the
standard Maven metadata file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a6b5d88-85fd-4ae4-97b1-68d93950d140
📒 Files selected for processing (2)
AGENTS.mddesign/registration/registration-upgrade.md
✅ Files skipped from review due to trivial changes (1)
- AGENTS.md
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
1 participant
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.
Summary by CodeRabbit