Add /typewriter extensions command to list extensions #386
Conversation
WalkthroughAdds a private “extensions” subcommand to Typewriter command to discover and list plugin extensions by scanning jars in the extensions directory, reading manifest JSONs, and determining enablement via namespace reflection or engineVersion matching. Results are sorted and rendered with hover details. Manifest command logic also gains inspect/page flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Cmd as TypewriterCommand
participant FS as Extensions Directory
participant Jar as Jar/Zip Reader
participant JSON as JSON Parser
participant RT as Runtime (Typewriter)
participant UI as Audience
User->>Cmd: /typewriter extensions
activate Cmd
note right of Cmd: Launch coroutine (IO)
Cmd->>FS: List *.jar (recursive)
FS-->>Cmd: Jar paths
loop For each jar
Cmd->>Jar: Open and read manifest entries<br/>(extension.json, blueprints.json, ...)
Jar-->>Cmd: Manifest content (if any)
Cmd->>JSON: Parse metadata (name, version, namespace, engineVersion)
JSON-->>Cmd: ExtMeta
alt Namespace present
Cmd->>RT: Reflect getExtensions()/loaded namespaces
RT-->>Cmd: Namespace found?
else engineVersion check
Cmd-->>Cmd: enabled = engineVersion matches
end
Cmd-->>Cmd: Collect JarInfo(displayName, version, namespace, enabled)
end
Cmd-->>Cmd: Sort by displayName
note right of Cmd: Switch to Sync dispatcher
Cmd->>UI: Render header + colored list with hover (author/namespace/version)
deactivate Cmd
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 2
🧹 Nitpick comments (4)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/command/TypewriterCommand.kt (4)
283-283: Line >120 chars; wrap to respect Kotlin style limit.Split this sendMini string for readability per guideline.
Example:
- sender.sendMini("<gray>- </gray><$color><hover:show_text:'$hover'>${info.displayName}</hover></$color>") + sender.sendMini( + "<gray>- </gray><$color><hover:show_text:'$hover'>${info.displayName}</hover></$color>" + )
315-344: Ensure UTF‑8 and close Reader, not just InputStream.Use UTF‑8 explicitly and .use on the Reader to avoid charset ambiguity and ensure both layers close.
Apply:
- val obj = zf.getInputStream(entry).use { JsonParser.parseReader(InputStreamReader(it)) }.asJsonObject + val obj = zf.getInputStream(entry).use { ins -> + InputStreamReader(ins, Charsets.UTF_8).use { r -> + JsonParser.parseReader(r).asJsonObject + } + }- val obj = zf.getInputStream(e).use { JsonParser.parseReader(InputStreamReader(it)) }.asJsonObject + val obj = zf.getInputStream(e).use { ins -> + InputStreamReader(ins, Charsets.UTF_8).use { r -> + JsonParser.parseReader(r).asJsonObject + } + }Add import (outside this hunk):
import kotlin.text.Charsets
345-360: Parsing is defensive and OK. Consider capturing display name if present.If manifests expose "name"/"displayName", surface it so list entries are friendlier than jar filenames.
If available, extend ExtMeta and set displayName from JSON, falling back to file name.
372-393: Reuse existing plugin reference instead of Bukkit lookup.You already have a global plugin; avoid null and redundant lookups.
-private fun tryDetectEnabledByRuntime(namespace: String?): Boolean? { +private fun tryDetectEnabledByRuntime(namespace: String?): Boolean? { if (namespace.isNullOrBlank()) return null - val plugin = org.bukkit.Bukkit.getPluginManager().getPlugin("Typewriter") ?: return null + val plugin = com.typewritermc.engine.paper.plugin
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/command/TypewriterCommand.kt(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt
📄 CodeRabbit inference engine (engine/AGENTS.md)
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt: Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin source files
Document public APIs with KDoc, focusing on when to use them rather than method-by-method descriptions
Avoid inline comments; refactor unclear code instead of adding explanations
Files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/command/TypewriterCommand.kt
🧠 Learnings (1)
📚 Learning: 2025-09-10T18:14:53.814Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: extensions/AGENTS.md:0-0
Timestamp: 2025-09-10T18:14:53.814Z
Learning: Applies to extensions/*Extension/build.gradle.kts : Each new extension must provide a build.gradle.kts that applies the "typewriter" plugin
Applied to files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/command/TypewriterCommand.kt
🧬 Code graph analysis (1)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/command/TypewriterCommand.kt (3)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/command/dsl/DslCommandTree.kt (3)
literal(45-49)literal(83-89)executes(37-39)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/command/dsl/DslCommand.kt (1)
literal(14-18)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/command/dsl/PaperDsl.kt (1)
withPermission(18-21)
🔇 Additional comments (6)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/command/TypewriterCommand.kt (6)
50-50: Good addition: registering the new subcommand.Hooking the new "extensions" subcommand into the main command tree is correct.
249-259: OK: permission-gated and guarded if folder missing.Permission "typewriter.extensions" and early return on missing folder look good.
362-365: Nice helper.Compact null‑safe accessor for primitives looks good.
398-412: Collection handling is fine. Minor: early return on first match.Loop already short‑circuits; implementation is acceptable.
414-432: Good: robust property resolution via method/field.This reflection helper covers common cases cleanly.
5-6: Check JSON library consistency across the repoFile: engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/command/TypewriterCommand.kt (lines 5–6) imports Gson.
Findings (samples):
- Gson used widely in engine modules (e.g. engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/Library.kt, engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/GenericSerializer.kt, engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/facts/storage/FileFactStorage.kt).
- kotlinx.serialization used in module-plugin (e.g. module-plugin/api/src/main/kotlin/com/typewritermc/moduleplugin/TypewriterModuleConfiguration.kt, module-plugin/processor/src/main/kotlin/com/typewritermc/processors/SharedJsonManager.kt and many processors).
Action required:
- Confirm whether mixed usage is intentional (Gson for engine modules, kotlinx for module-plugin) or if the project should standardize.
- If not intentional, standardize or isolate the two libraries by module (recommend keeping a single JSON library per module boundary).
| infos.forEach { info -> | ||
| val color = if (info.enabled == true) "green" else "red" | ||
| val hover = buildString { | ||
| append("author: ${info.namespace ?: "unknown"}\n") | ||
| append("version: ${info.version ?: "unknown"}") | ||
| }.replace("'", "\\'") | ||
| sender.sendMini("<gray>- </gray><$color><hover:show_text:'$hover'>${info.displayName}</hover></$color>") | ||
| } | ||
| } |
There was a problem hiding this comment.
Enabled vs. compatible conflation; fix status + hover label.
Runtime-negative should not be flipped to “enabled” just because engineVersion matches. Report enabled only when runtime-detected; otherwise “unknown”. Also the hover says "author" but shows the namespace.
Apply:
- val color = if (info.enabled == true) "green" else "red"
- val hover = buildString {
- append("author: ${info.namespace ?: "unknown"}\n")
- append("version: ${info.version ?: "unknown"}")
- }.replace("'", "\\'")
+ val color = when (info.enabled) {
+ true -> "green"
+ false -> "red"
+ null -> "yellow"
+ }
+ val statusText = when (info.enabled) {
+ true -> "enabled"
+ false -> "disabled"
+ null -> "unknown"
+ }
+ val hover = buildString {
+ append("namespace: ${info.namespace ?: "unknown"}\n")
+ append("version: ${info.version ?: "unknown"}\n")
+ append("status: $statusText")
+ }.replace("'", "\\'")-private fun detectEnabled(meta: ExtMeta?, engineVersion: String): Boolean {
- if (meta == null) return false
- tryDetectEnabledByRuntime(meta.namespace)?.let { return it }
- return !meta.engineVersion.isNullOrBlank() && meta.engineVersion == engineVersion
+private fun detectEnabled(meta: ExtMeta?, engineVersion: String): Boolean? {
+ if (meta == null) return null
+ val runtime = tryDetectEnabledByRuntime(meta.namespace)
+ if (runtime != null) return runtime
+ // Unknown without reliable runtime signal
+ return null
}Also applies to: 366-371
| private fun invokeNoArg(target: Any, method: String): Any? = | ||
| try { target.javaClass.getMethod(method).apply { isAccessible = true }.invoke(target) } catch (_: Throwable) { null } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Broaden reflection: fall back to declared methods.
Current code only sees public no‑arg methods; use getDeclaredMethod as a fallback.
Apply:
-private fun invokeNoArg(target: Any, method: String): Any? =
- try { target.javaClass.getMethod(method).apply { isAccessible = true }.invoke(target) } catch (_: Throwable) { null }
+private fun invokeNoArg(target: Any, method: String): Any? {
+ val cls = target.javaClass
+ return try {
+ cls.getMethod(method).invoke(target)
+ } catch (_: Throwable) {
+ try {
+ cls.getDeclaredMethod(method).apply { isAccessible = true }.invoke(target)
+ } catch (_: Throwable) {
+ null
+ }
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun invokeNoArg(target: Any, method: String): Any? = | |
| try { target.javaClass.getMethod(method).apply { isAccessible = true }.invoke(target) } catch (_: Throwable) { null } | |
| private fun invokeNoArg(target: Any, method: String): Any? { | |
| val cls = target.javaClass | |
| return try { | |
| cls.getMethod(method).invoke(target) | |
| } catch (_: Throwable) { | |
| try { | |
| cls.getDeclaredMethod(method).apply { isAccessible = true }.invoke(target) | |
| } catch (_: Throwable) { | |
| null | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/command/TypewriterCommand.kt
around lines 395 to 397, the helper only uses target.javaClass.getMethod(method)
which only finds public no-arg methods; update it to first try getMethod(method)
and, if that throws NoSuchMethodException, fall back to
target.javaClass.getDeclaredMethod(method), set it accessible, then invoke; keep
the existing try/catch to return null on failure and ensure setAccessible(true)
is applied to the resolved Method before invoking.
|
|
||
| Dispatchers.UntickedAsync.launch { | ||
| val jars = findJarFilesRecursive(extensionsDir) | ||
| val infos = jars.map { f -> |
There was a problem hiding this comment.
This is something I've noticed throughout the PR. I think it would be better to use full parameter names instead of abbreviated ones. For example, instead of f, we could use file, or instead of zf, we could use zipFile.
| return out | ||
| } | ||
|
|
||
| private fun readExtensionMetaFromJar(jar: File): ExtMeta? = try { |
There was a problem hiding this comment.
As discussed on Discord, instead of reflection we should just get the extension loader instance with koin.
|
@iamyellowhead Are you ever going to make the changes or not? Otherwise I will just close this pr |
iamyellowhead
force-pushed
the
develop
branch
from
b1cf8dc to
66aa073
Compare
3 participants
Adds an extensions subcommand to typewriter that lists all extension JARs, shows their metadata, and marks whether each is enabled. It supports recursive JAR discovery, JSON metadata extraction, and runtime namespace checks.
Summary by CodeRabbit