feat: migrate opsx from Gradle plugin to standalone CLI + TUI (v0.50.0)#9
Conversation
|
Important Review skippedToo many files! This PR contains 279 files, which is 129 over the limit of 150. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (279)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughGenerate per-agent SKILL.md and three agent markdowns (opsx/srcx/wrkx) directly into each agent’s project directories, remove shared Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,230,201,0.5)
participant Gradle as Gradle Task
participant Gen as SkillGenerator
participant AgentStore as Agent Dir (project)
participant AgentProc as Agent CLI Process
end
Gradle->>Gen: invoke generateSkillFiles(activeAgents, tasks, additionalDirs)
Gen->>AgentStore: write `skillsDir/<skill>/SKILL.md` and `opsx.md/srcx.md/wrkx.md`
Gradle->>AgentProc: buildCommand(agent, prompt, model) -> spawn CLI
AgentProc-->>Gradle: stream stdout/stderr
Gradle->>Gradle: logger.quiet(...) logs output (no per-agent log files)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/test/kotlin/zone/clanker/opsx/workflow/AgentDetectorTest.kt (1)
69-80: Consider adding tests fordetectFromProcessTreefallback.The test verifies env detection succeeds but doesn't test the fallback path when env detection returns null and process tree detection is attempted. While process tree detection is harder to unit test (would require mocking
ProcessHandle), consider adding at least a test that verifiesdetect()returnsnullwhen both detection methods fail.Example additional test case
`when`("env detection fails and no process tree match") { val result = AgentDetector.detect( listOf(Agent.CLAUDE, Agent.COPILOT), ) { null } then("returns null") { // Process tree unlikely to match in test environment // This at least exercises the fallback path result shouldBe null } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/zone/clanker/opsx/workflow/AgentDetectorTest.kt` around lines 69 - 80, Add a unit test that exercises the detect() fallback path by calling AgentDetector.detect with the same candidate list (e.g., listOf(Agent.CLAUDE, Agent.COPILOT)) but supplying an env resolver that returns null so env detection fails; assert the result is null to verify that when detectFromProcessTree also finds nothing detect() returns null (this will at least invoke the fallback logic without needing to mock ProcessHandle).src/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.kt (1)
236-244: Minor redundancy in file reference.Line 237 writes to
File(agentSourceDir, filename)and line 238 creates the sameFileobject again assourceFile. Consider reusing the file reference.♻️ Suggested simplification
GENERATED_AGENT_FILES.forEach { filename -> - File(agentSourceDir, filename).writeText(buildAgentFile(filename, agent)) val sourceFile = File(agentSourceDir, filename) + sourceFile.writeText(buildAgentFile(filename, agent)) if (agent.usesCopy) { sourceFile.copyTo(File(projectDir, filename), overwrite = true) } else { createSymlink(File(projectDir, filename), sourceFile) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.kt` around lines 236 - 244, The code creates the same File(agentSourceDir, filename) twice; instead instantiate a single val sourceFile = File(agentSourceDir, filename) before calling buildAgentFile/writeText and then reuse that sourceFile for the copyTo or createSymlink branch (inside the loop over GENERATED_AGENT_FILES). Update the loop that references GENERATED_AGENT_FILES, agentSourceDir, filename, buildAgentFile, sourceFile, agent.usesCopy, projectDir, and createSymlink to avoid recreating the File object.src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt (2)
11-19: Consider extracting sharedwithHomehelper to reduce duplication.This
withHomefunction is duplicated fromSkillGeneratorAgentTest.kt. Consider extracting it to a shared test utility file to follow DRY principles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt` around lines 11 - 19, Extract the duplicated helper function withHome into a shared test utility (e.g., TestUtils or TestHelpers) and replace the copies in SkillDistributionMatrixTest and SkillGeneratorAgentTest with calls to that shared helper; move the implementation (preserving behavior of capturing and restoring System.getProperty("user.home")) into the new utility as a public function and update both tests to import and use it to remove duplication.
21-26:deleteOnExit()may not clean up directories with contents.The
deleteOnExit()method only deletes empty directories. Since tests create files inside, these directories may persist after test runs. Consider using JUnit's@TempDiror Kotest'stempdir()extension for reliable cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt` around lines 21 - 26, The tempDir function in SkillDistributionMatrixTest (private fun tempDir) uses File.deleteOnExit(), which won't remove non-empty dirs, causing test artifacts to persist; replace this helper with a proper temporary-directory facility (e.g., JUnit's `@TempDir` field/parameter or Kotest's tempdir() extension) and update tests to use the provided Path/File from `@TempDir` or tempdir(), removing the custom tempDir function and any manual deleteOnExit() calls so the test framework handles reliable cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/zone/clanker/opsx/workflow/AgentDetector.kt`:
- Around line 28-38: The process basename extraction in detectFromProcessTree
yields names like "claude.exe" on Windows so it never matches configured
cliCommand values; change the extraction at the `name` assignment in
detectFromProcessTree (inside AgentDetector) to strip a trailing ".exe" (e.g.,
apply removeSuffix(".exe") to the basename) before looking it up in `byCommand`
so "claude.exe" will match "claude".
In `@src/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorAgentTest.kt`:
- Around line 138-140: Update the COPILOT test assertion so it checks the actual
file path instead of only the directory: replace the weak check in the test
block labeled then("writes source of truth to ~/.clkx/agents/opsx.md") that uses
File(tempDir, ".clkx/agents").exists() with an assertion that File(tempDir,
".clkx/agents/copilot/opsx.md").exists() shouldBe true so the test verifies the
specific file was created.
- Around line 182-184: The test currently asserts existence of
".clkx/agents/opsx.md" but CODEX uses a per-agent subdirectory; update the
assertion in SkillGeneratorAgentTest (the "then(\"writes source of truth to
~/.clkx/agents/opsx.md\")" block) to check for ".clkx/agents/codex/opsx.md"
instead (i.e. assert File(tempDir, ".clkx/agents/codex/opsx.md").exists()
shouldBe true) so the test matches the new per-agent layout.
- Around line 220-222: The test currently only asserts that the ".clkx/agents"
directory exists; update the assertion in the then block (test named "writes
source of truth to ~/.clkx/agents/opsx.md") to specifically verify the OPENCODE
output file exists by checking ".clkx/agents/opencode/opsx.md" (e.g. replace the
File(tempDir, ".clkx/agents").exists() assertion with a check that File(tempDir,
".clkx/agents/opencode/opsx.md").exists() is true).
- Around line 96-98: The test's assertion in SkillGeneratorAgentTest (the then
block "writes source of truth to ~/.clkx/agents/opsx.md") currently only checks
the directory exists; change it to assert the concrete file is created (e.g.,
assert File(tempDir, ".clkx/agents/claude/opsx.md").exists() or derive the agent
subfolder/name used by the test) so the test verifies the actual opsx.md file is
written; update the assertion in the then block accordingly to check the
specific file path instead of just the directory.
---
Nitpick comments:
In `@src/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.kt`:
- Around line 236-244: The code creates the same File(agentSourceDir, filename)
twice; instead instantiate a single val sourceFile = File(agentSourceDir,
filename) before calling buildAgentFile/writeText and then reuse that sourceFile
for the copyTo or createSymlink branch (inside the loop over
GENERATED_AGENT_FILES). Update the loop that references GENERATED_AGENT_FILES,
agentSourceDir, filename, buildAgentFile, sourceFile, agent.usesCopy,
projectDir, and createSymlink to avoid recreating the File object.
In `@src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt`:
- Around line 11-19: Extract the duplicated helper function withHome into a
shared test utility (e.g., TestUtils or TestHelpers) and replace the copies in
SkillDistributionMatrixTest and SkillGeneratorAgentTest with calls to that
shared helper; move the implementation (preserving behavior of capturing and
restoring System.getProperty("user.home")) into the new utility as a public
function and update both tests to import and use it to remove duplication.
- Around line 21-26: The tempDir function in SkillDistributionMatrixTest
(private fun tempDir) uses File.deleteOnExit(), which won't remove non-empty
dirs, causing test artifacts to persist; replace this helper with a proper
temporary-directory facility (e.g., JUnit's `@TempDir` field/parameter or Kotest's
tempdir() extension) and update tests to use the provided Path/File from
`@TempDir` or tempdir(), removing the custom tempDir function and any manual
deleteOnExit() calls so the test framework handles reliable cleanup.
In `@src/test/kotlin/zone/clanker/opsx/workflow/AgentDetectorTest.kt`:
- Around line 69-80: Add a unit test that exercises the detect() fallback path
by calling AgentDetector.detect with the same candidate list (e.g.,
listOf(Agent.CLAUDE, Agent.COPILOT)) but supplying an env resolver that returns
null so env detection fails; assert the result is null to verify that when
detectFromProcessTree also finds nothing detect() returns null (this will at
least invoke the fallback logic without needing to mock ProcessHandle).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18a42ff3-9838-4d64-a189-ed851b1c0c9e
📒 Files selected for processing (15)
src/main/kotlin/zone/clanker/opsx/Opsx.ktsrc/main/kotlin/zone/clanker/opsx/model/Agent.ktsrc/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.ktsrc/main/kotlin/zone/clanker/opsx/task/CleanTask.ktsrc/main/kotlin/zone/clanker/opsx/task/SyncTask.ktsrc/main/kotlin/zone/clanker/opsx/workflow/AgentDetector.ktsrc/main/kotlin/zone/clanker/opsx/workflow/AgentDispatcher.ktsrc/test/kotlin/zone/clanker/opsx/OpsxPluginSyncTest.ktsrc/test/kotlin/zone/clanker/opsx/OpsxPluginTest.ktsrc/test/kotlin/zone/clanker/opsx/model/AgentTest.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorAgentTest.ktsrc/test/kotlin/zone/clanker/opsx/task/CleanTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/workflow/AgentDetectorTest.ktsrc/test/kotlin/zone/clanker/opsx/workflow/AgentDispatcherTest.kt
✅ Files skipped from review due to trivial changes (2)
- src/main/kotlin/zone/clanker/opsx/model/Agent.kt
- src/test/kotlin/zone/clanker/opsx/model/AgentTest.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/kotlin/zone/clanker/opsx/workflow/AgentDispatcherTest.kt
- src/main/kotlin/zone/clanker/opsx/workflow/AgentDispatcher.kt
| internal fun detectFromProcessTree(configured: List<Agent>): Agent? { | ||
| val byCommand = configured.associateBy { it.cliCommand } | ||
| var handle: ProcessHandle? = ProcessHandle.current().parent().orElse(null) | ||
| while (handle != null) { | ||
| val cmd = handle.info().command().orElse(null) ?: break | ||
| val name = cmd.substringAfterLast("/").substringAfterLast("\\") | ||
| byCommand[name]?.let { return it } | ||
| handle = handle.parent().orElse(null) | ||
| } | ||
| return null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Agent.cliCommand values don't include extensions
ast-grep --pattern 'enum class Agent($$$) {
$$$
}'Repository: ClankerGuru/opsx
Length of output: 42
🏁 Script executed:
fd -t f "Agent.kt" -o -name "*Agent*"Repository: ClankerGuru/opsx
Length of output: 290
🏁 Script executed:
rg "enum class Agent|data class Agent|class Agent" -A 5Repository: ClankerGuru/opsx
Length of output: 2867
🏁 Script executed:
cat src/main/kotlin/zone/clanker/opsx/model/Agent.ktRepository: ClankerGuru/opsx
Length of output: 1441
Strip .exe extension from process command name for Windows compatibility.
On Windows, ProcessHandle.info().command() returns paths like C:\...\claude.exe. The basename extraction yields claude.exe, which won't match cliCommand values ("claude", "copilot", etc.). Add .removeSuffix(".exe") to the basename extraction:
Proposed fix
- val name = cmd.substringAfterLast("/").substringAfterLast("\\")
+ val name = cmd.substringAfterLast("/").substringAfterLast("\\")
+ .removeSuffix(".exe")📝 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.
| internal fun detectFromProcessTree(configured: List<Agent>): Agent? { | |
| val byCommand = configured.associateBy { it.cliCommand } | |
| var handle: ProcessHandle? = ProcessHandle.current().parent().orElse(null) | |
| while (handle != null) { | |
| val cmd = handle.info().command().orElse(null) ?: break | |
| val name = cmd.substringAfterLast("/").substringAfterLast("\\") | |
| byCommand[name]?.let { return it } | |
| handle = handle.parent().orElse(null) | |
| } | |
| return null | |
| } | |
| internal fun detectFromProcessTree(configured: List<Agent>): Agent? { | |
| val byCommand = configured.associateBy { it.cliCommand } | |
| var handle: ProcessHandle? = ProcessHandle.current().parent().orElse(null) | |
| while (handle != null) { | |
| val cmd = handle.info().command().orElse(null) ?: break | |
| val name = cmd.substringAfterLast("/").substringAfterLast("\\") | |
| .removeSuffix(".exe") | |
| byCommand[name]?.let { return it } | |
| handle = handle.parent().orElse(null) | |
| } | |
| return null | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/zone/clanker/opsx/workflow/AgentDetector.kt` around lines 28
- 38, The process basename extraction in detectFromProcessTree yields names like
"claude.exe" on Windows so it never matches configured cliCommand values; change
the extraction at the `name` assignment in detectFromProcessTree (inside
AgentDetector) to strip a trailing ".exe" (e.g., apply removeSuffix(".exe") to
the basename) before looking it up in `byCommand` so "claude.exe" will match
"claude".
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/kotlin/zone/clanker/opsx/task/CleanTask.kt (1)
149-151: Avoid duplicated generated-agent filename lists across modules.
AGENT_FILEShere duplicates the generated file set already defined insrc/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.kt(seegenerateAgentDefinitionsusage). Please centralize this list to a shared constant to prevent drift.♻️ Suggested direction
- companion object { - private val AGENT_FILES = listOf("opsx.md", "srcx.md", "wrkx.md") - } + companion object { + // Reuse a shared constant (e.g., from SkillGenerator or a common constants object) + private val AGENT_FILES = SkillGenerator.GENERATED_AGENT_FILES + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/zone/clanker/opsx/task/CleanTask.kt` around lines 149 - 151, The companion object constant AGENT_FILES in CleanTask duplicates the generated-agent filename list defined in SkillGenerator (used by generateAgentDefinitions); remove the duplicate here and reference a single shared constant instead (e.g., add or export a public constant in SkillGenerator or a new shared object like GeneratedAgentFiles) then update CleanTask to use that shared constant (replace AGENT_FILES with the shared symbol) and adjust imports; ensure only one authoritative list exists and delete the old AGENT_FILES declaration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt`:
- Around line 199-200: The test in SkillDistributionMatrixTest (the then block
asserting File(dir, ".clkx").exists() shouldBe false) only checks a
project-local path and doesn't ensure the real $HOME/.clkx wasn't written;
update the test to set a temporary isolated home by overriding the JVM system
property "user.home" (or create a separate test that inspects
Paths.get(System.getProperty("user.home"), ".clkx")) before running the code
under test, run the generation, and then assert the temp-home-derived path does
not exist (and restore the original "user.home" afterwards) so the actual
home-derived path is verified instead of just the project-local dir.
- Around line 62-66: The test in SkillDistributionMatrixTest claims to ensure
"skills are real files (not symlinks)" but only uses file.isFile; update the
assertion to explicitly check the path is not a symbolic link by converting the
File to a Path and asserting Files.isSymbolicLink(file.toPath()) is false for
each constructed File (use the same file variable built from agent.skillsDir and
task.name/SKILL.md); keep the existing isFile assertion and add this negative
symlink assertion so the test fails if a symlink was used.
In `@src/test/kotlin/zone/clanker/opsx/workflow/AgentDetectorTest.kt`:
- Around line 76-88: Add a new test in AgentDetectorTest that covers the
fallback path by ensuring detectFromEnv returns null and the process-tree path
is exercised: call AgentDetector.detect(...) with an env-mapping lambda that
returns null for the relevant keys and arrange for detectFromProcessTree
(indirectly exercised by AgentDetector.detect) to identify a specific agent
(e.g., Agent.COPILOT); then assert the returned value equals that agent.
Reference the top-level function AgentDetector.detect and its helper behaviors
detectFromEnv / detectFromProcessTree and use concrete Agent enum values
(Agent.CLAUDE, Agent.COPILOT) so the test verifies the fallback branch.
---
Nitpick comments:
In `@src/main/kotlin/zone/clanker/opsx/task/CleanTask.kt`:
- Around line 149-151: The companion object constant AGENT_FILES in CleanTask
duplicates the generated-agent filename list defined in SkillGenerator (used by
generateAgentDefinitions); remove the duplicate here and reference a single
shared constant instead (e.g., add or export a public constant in SkillGenerator
or a new shared object like GeneratedAgentFiles) then update CleanTask to use
that shared constant (replace AGENT_FILES with the shared symbol) and adjust
imports; ensure only one authoritative list exists and delete the old
AGENT_FILES declaration.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e7f15d2f-d9f1-4350-870a-01f3ca72ea2f
📒 Files selected for processing (13)
src/main/kotlin/zone/clanker/opsx/Opsx.ktsrc/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.ktsrc/main/kotlin/zone/clanker/opsx/task/CleanTask.ktsrc/main/kotlin/zone/clanker/opsx/task/SyncTask.ktsrc/main/kotlin/zone/clanker/opsx/workflow/AgentDetector.ktsrc/test/kotlin/zone/clanker/opsx/OpsxPluginSyncTest.ktsrc/test/kotlin/zone/clanker/opsx/OpsxPluginTest.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorAgentTest.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorTest.ktsrc/test/kotlin/zone/clanker/opsx/task/CleanTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/task/SyncTaskTest.ktsrc/test/kotlin/zone/clanker/opsx/workflow/AgentDetectorTest.kt
💤 Files with no reviewable changes (1)
- src/test/kotlin/zone/clanker/opsx/task/SyncTaskTest.kt
✅ Files skipped from review due to trivial changes (1)
- src/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.kt
🚧 Files skipped from review as they are similar to previous changes (7)
- src/main/kotlin/zone/clanker/opsx/Opsx.kt
- src/test/kotlin/zone/clanker/opsx/OpsxPluginSyncTest.kt
- src/main/kotlin/zone/clanker/opsx/workflow/AgentDetector.kt
- src/test/kotlin/zone/clanker/opsx/task/CleanTaskTest.kt
- src/test/kotlin/zone/clanker/opsx/OpsxPluginTest.kt
- src/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorTest.kt
- src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt
| then("active ${agent.id}: skills are real files (not symlinks)") { | ||
| sampleTasks.forEach { task -> | ||
| val file = File(dir, "${agent.skillsDir}/${task.name}/SKILL.md") | ||
| file.isFile shouldBe true | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt | sed -n '50,75p'Repository: ClankerGuru/opsx
Length of output: 1381
🏁 Script executed:
cat -n src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt | sed -n '195,210p'Repository: ClankerGuru/opsx
Length of output: 451
🏁 Script executed:
head -30 src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.ktRepository: ClankerGuru/opsx
Length of output: 1017
🌐 Web query:
java File.isFile() symlinks behavior follows
💡 Result:
The java.io.File.isFile method follows symbolic links (symlinks). It transparently redirects to the target of the symlink and tests whether the target is a normal (regular) file. Key behaviors: - If the symlink points to a regular file, isFile returns true. - If the symlink points to a directory, isFile returns false (use isDirectory instead, which also follows symlinks). - If the symlink is broken (target does not exist), isFile returns false (and exists also returns false). - Symlinks themselves are not considered "normal files"; the method checks the target's type after dereferencing. This is per the official Java documentation: "Unless otherwise specified, symbolic links are transparent to applications and operations on files that are symbolic links are automatically redirected to the target of the link." File.isFile is such an operation that accesses the filesystem. For symlink-aware checks without following links, use java.nio.file.Files.isRegularFile(path, LinkOption.NOFOLLOW_LINKS), which returns false for symlinks regardless of target.
Citations:
- 1: https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/io/File.html
- 2: https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/io/File.html
- 3: https://docs.oracle.com/javase/8/docs/api/java/io/File.html
- 4: https://stackoverflow.com/questions/28371993/resolving-directory-symlink-in-java
- 5: https://stackoverflow.com/questions/2175673/java-check-symbolic-link-file-existence
🌐 Web query:
java Files.isSymbolicLink() how to check if path is symlink
💡 Result:
To check if a path is a symbolic link in Java, use the static method java.nio.file.Files.isSymbolicLink(Path path). This method returns true if the file is a symbolic link, false if the file does not exist, is not a symbolic link, or it cannot be determined. Example usage: import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; Path path = Paths.get("/path/to/file"); boolean isSymlink = Files.isSymbolicLink(path); System.out.println("Is symbolic link: " + isSymlink); This method is part of the NIO.2 API (Java 7+) and does not follow the link—it checks the path itself. It may throw SecurityException if access is denied, but typically no IOException for the check itself. Note: BasicFileAttributes.isSymbolicLink (from Files.readAttributes) may differ if links are followed by default; use LinkOption.NOFOLLOW_LINKS to match Files.isSymbolicLink behavior. To read the symlink target, use Files.readSymbolicLink(path).
Citations:
- 1: https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/nio/file/Files.html
- 2: https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html
- 3: https://www.logicbig.com/how-to/code-snippets/jcode-java-io-files-issymboliclink.html
- 4: https://download.oracle.com/javase/tutorial/essential/io/links.html
- 5: https://docs.oracle.com/javase/tutorial/essential/io/links.html
🌐 Web query:
java Files.isRegularFile LinkOption.NOFOLLOW_LINKS vs isFile isSymbolicLink
💡 Result:
In Java NIO.2 (java.nio.file.Files), there is no Files.isFile method. The query likely refers to java.io.File.isFile for comparison. Files.isRegularFile(Path path, LinkOption... options): Tests if the path is a regular file (not directory, symlink, or special file like device/socket). By default (no options), follows symlinks: returns true if symlink target is regular file. With LinkOption.NOFOLLOW_LINKS, does not follow symlinks: returns true only if path itself is regular file (symlink returns false). Files.isSymbolicLink(Path path): Tests if path is a symbolic link. Does not follow symlinks (always examines the path itself). No LinkOption parameter. java.io.File.isFile: Tests if path denotes a normal file (not directory). Follows symlinks transparently (returns true if target is normal file). Less precise than isRegularFile (may return true for some special files). Key differences with NOFOLLOW_LINKS: - isRegularFile without option: follows links, checks target type. - isRegularFile(NOFOLLOW_LINKS): checks path itself, ignores target. - isSymbolicLink: always checks path itself for being a symlink. - isFile (java.io.File): always follows links. To check if path is regular file not symlink: !Files.isSymbolicLink(path) && Files.isRegularFile(path, NOFOLLOW_LINKS). Or Files.isRegularFile(path, NOFOLLOW_LINKS) alone (since symlinks aren't regular files).
Citations:
- 1: https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/nio/file/Files.html
- 2: https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/nio/file/Files.html
- 3: https://medium.com/@AlexanderObregon/javas-files-isregularfile-method-explained-bcf776caa8eb
- 4: https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html
- 5: https://stackoverflow.com/questions/55894781/which-methods-of-java-nio-file-files-follow-symbolic-links-and-which-dont
- 6: https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/io/File.html
- 7: https://docs.oracle.com/javase/tutorial/essential/io/links.html
🏁 Script executed:
cat -n src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt | sed -n '25,50p'Repository: ClankerGuru/opsx
Length of output: 1248
🏁 Script executed:
grep -n "val dir" src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt | head -5Repository: ClankerGuru/opsx
Length of output: 113
🏁 Script executed:
cat -n src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt | wc -lRepository: ClankerGuru/opsx
Length of output: 63
File.isFile() does not verify files are not symlinks; add explicit check.
The test name at line 62 claims to verify "skills are real files (not symlinks)", but line 65 only checks file.isFile shouldBe true. This passes even if the file is a symlink pointing to a regular file. To actually prevent symlink-based distribution, explicitly assert that the path is not a symbolic link.
Suggested fix
import io.kotest.matchers.string.shouldContain
import io.kotest.matchers.string.shouldNotContain
import zone.clanker.opsx.model.Agent
import java.io.File
+import java.nio.file.Files
then("active ${agent.id}: skills are real files (not symlinks)") {
sampleTasks.forEach { task ->
val file = File(dir, "${agent.skillsDir}/${task.name}/SKILL.md")
file.isFile shouldBe true
+ Files.isSymbolicLink(file.toPath()) shouldBe false
}
}📝 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.
| then("active ${agent.id}: skills are real files (not symlinks)") { | |
| sampleTasks.forEach { task -> | |
| val file = File(dir, "${agent.skillsDir}/${task.name}/SKILL.md") | |
| file.isFile shouldBe true | |
| } | |
| then("active ${agent.id}: skills are real files (not symlinks)") { | |
| sampleTasks.forEach { task -> | |
| val file = File(dir, "${agent.skillsDir}/${task.name}/SKILL.md") | |
| file.isFile shouldBe true | |
| Files.isSymbolicLink(file.toPath()) shouldBe false | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt`
around lines 62 - 66, The test in SkillDistributionMatrixTest claims to ensure
"skills are real files (not symlinks)" but only uses file.isFile; update the
assertion to explicitly check the path is not a symbolic link by converting the
File to a Path and asserting Files.isSymbolicLink(file.toPath()) is false for
each constructed File (use the same file variable built from agent.skillsDir and
task.name/SKILL.md); keep the existing isFile assertion and add this negative
symlink assertion so the test fails if a symlink was used.
| then("no ~/.clkx/ directory created") { | ||
| File(dir, ".clkx").exists() shouldBe false |
There was a problem hiding this comment.
This does not verify that $HOME/.clkx stays untouched.
File(dir, ".clkx") only checks for a project-local legacy directory. If generation regressed and wrote to the real home directory again, this assertion would still pass. Point user.home at an isolated temp home for this case, or add a dedicated test around the actual home-derived path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/test/kotlin/zone/clanker/opsx/skill/SkillDistributionMatrixTest.kt`
around lines 199 - 200, The test in SkillDistributionMatrixTest (the then block
asserting File(dir, ".clkx").exists() shouldBe false) only checks a
project-local path and doesn't ensure the real $HOME/.clkx wasn't written;
update the test to set a temporary isolated home by overriding the JVM system
property "user.home" (or create a separate test that inspects
Paths.get(System.getProperty("user.home"), ".clkx")) before running the code
under test, run the generation, and then assert the temp-home-derived path does
not exist (and restore the original "user.home" afterwards) so the actual
home-derived path is verified instead of just the project-local dir.
| given("detect") { | ||
|
|
||
| `when`("env detection succeeds") { | ||
| val env = mapOf("CLAUDE_CODE_ENTRY_POINT" to "cli") | ||
| val result = | ||
| AgentDetector.detect( | ||
| listOf(Agent.CLAUDE, Agent.COPILOT), | ||
| ) { env[it] } | ||
| then("returns env result without checking process tree") { | ||
| result shouldBe Agent.CLAUDE | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Process-tree fallback is still untested.
This only proves the env-hit branch of detect(). If detectFromEnv(...) returns null, the new detectFromProcessTree(...) path never runs anywhere in this suite, so the core fallback behavior can regress unnoticed. Please add an env-miss case that exercises the process-tree branch as well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/test/kotlin/zone/clanker/opsx/workflow/AgentDetectorTest.kt` around lines
76 - 88, Add a new test in AgentDetectorTest that covers the fallback path by
ensuring detectFromEnv returns null and the process-tree path is exercised: call
AgentDetector.detect(...) with an env-mapping lambda that returns null for the
relevant keys and arrange for detectFromProcessTree (indirectly exercised by
AgentDetector.detect) to identify a specific agent (e.g., Agent.COPILOT); then
assert the returned value equals that agent. Reference the top-level function
AgentDetector.detect and its helper behaviors detectFromEnv /
detectFromProcessTree and use concrete Agent enum values (Agent.CLAUDE,
Agent.COPILOT) so the test verifies the fallback branch.
f333f88 to
b4ce0ee
Compare
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
b4ce0ee to
5c8845e
Compare
Replaces the opsx Gradle plugin with a standalone Kotlin CLI application distributed as a shadow JAR. The new architecture splits into three modules: - cli/ — headless domain logic organized by feature (init, install, update, nuke, status, log, list) - tui/ — full-screen interactive TUI with Mordant widgets, gum-style confirm dialogs, resize support - app/ — thin entry point wiring CLI commands and TUI dashboard Key changes: - Full-screen TUI with ASCII logo, gradient colors, SelectList menu, vim keys (j/k/h/l) - Init writes skills/agents directly to host directories (.claude/, .github/, .codex-plugin/, .opencode/) - Nuke selectively removes only opsx-owned files using manifest, preserving user content - Install copies binary to ~/.opsx/bin with PATH wiring and zsh completions - Update checks GitHub releases with download, checksum verification, and extraction - All file I/O via kotlinx-io (Path + SystemFileSystem), java.io.File forbidden - Convention plugins: clkx-kotlin, clkx-cli, clkx-app, clkx-lib, clkx-detekt, clkx-ktlint, clkx-kover, clkx-konsist - 95%+ test coverage on both cli and tui modules - Skills embedded as JAR resources including mordant-tui, kotlinx-io, okio references - Reusable ConfirmDialog component matching Charmbracelet gum confirm UX - AppShell shared layout (logo, content, status bar) across all screens
5c8845e to
b72c75b
Compare
Summary
Complete rewrite of opsx from a Gradle settings plugin to a standalone Kotlin CLI application with a full-screen TUI.
What changed
opsx initwrites skills/agents directly to host directories (.claude/, .github/, .codex-plugin/, .opencode/)opsx nukeselectively removes only opsx-owned files using the manifest, preserving user contentopsx installcopies binary to ~/.opsx/bin with PATH wiring and zsh completionsopsx updatechecks GitHub releases with download, checksum verification, and extractionRelease
Tag as
v0.50.0on merge. Version in gradle.properties is0.1.0-SNAPSHOTfor development; the release workflow uses the git tag.Test plan
./gradlew clean check --no-build-cachepasses (compile, detekt, ktlint, tests, kover)opsx init --host claude --host copilot --host codex --host opencodeinstalls to correct directoriesopsx nuke --keep-rcremoves only opsx-owned filesopsx installcopies to ~/.opsx/bin, wires PATHopsx updatechecks GitHub releases