Filter symlinks by agent, expand instructions, generate opsx agent#4
Merged
Conversation
- SkillGenerator accepts defaultAgent, only generates symlinks for the configured agent instead of all 4 - Instruction files (CLAUDE.md/AGENTS.md) now include full onboarding: what opsx is, how srcx context works, change lifecycle, strict rules - Generate selectable opsx agent at .claude/agents/opsx.md and .github/agents/opsx.md with lifecycle knowledge and rules - SyncTask wires defaultAgent from extension
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 13 minutes and 27 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughPropagates a defaultAgent from the Opsx extension into the Changes
Sequence DiagramsequenceDiagram
participant Gradle as Gradle Task\n(opsx-sync)
participant SyncTask as SyncTask
participant SkillGen as SkillGenerator
participant FS as FileSystem
Gradle->>SyncTask: run() with defaultAgent
SyncTask->>SkillGen: construct(defaultAgent)
SyncTask->>SkillGen: request activeTarget()
alt known agent
SkillGen-->>SyncTask: AgentTarget(skillDir,instructionFile,agentDir)
SyncTask->>SkillGen: generateSkillFiles()
SkillGen->>FS: create symlinks in active skillDir only
SyncTask->>SkillGen: generateInstructionFiles()
SkillGen->>FS: write AGENTS.md and agent-specific file (if any)
SyncTask->>SkillGen: generateAgentDefinitions()
SkillGen->>FS: write opsx.md to active agentDir (if applicable)
SyncTask->>FS: cleanup symlinks/.gitignore in active agent dirs only
else unknown agent
SkillGen-->>SyncTask: throw error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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.
🧹 Nitpick comments (1)
src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt (1)
99-109: Consider handling broken symlinks inisOpsxSymlink.The
runCatchingblock returnsfalseon exception, which is reasonable. However,Files.readSymbolicLinkdoesn't resolve the target—it returns the raw path stored in the symlink. If the symlink was created with an absolute path, the comparisontarget.startsWith(sourceDir.toPath())works correctly. But if it was created with a relative path, the comparison may fail unexpectedly.The current implementation is likely fine since
generateSkillFilescreates symlinks using absolute paths (source.toPath()), but this is worth noting if the behavior changes.🤖 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/SyncTask.kt` around lines 99 - 109, isOpsxSymlink currently reads the raw symlink target via Files.readSymbolicLink and compares it directly to sourceDir, which fails for relative symlinks; update isOpsxSymlink to resolve the read target against the symlink's parent (e.g., path.parent.resolve(target).normalize()) before comparing with sourceDir.toPath(), and keep the existing runCatching/exception-to-false behavior to treat broken/unresolvable symlinks as non-opsx symlinks; note generateSkillFiles creates absolute links today but ensure the new resolution covers relative links too.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/kotlin/zone/clanker/opsx/task/SyncTask.kt`:
- Around line 99-109: isOpsxSymlink currently reads the raw symlink target via
Files.readSymbolicLink and compares it directly to sourceDir, which fails for
relative symlinks; update isOpsxSymlink to resolve the read target against the
symlink's parent (e.g., path.parent.resolve(target).normalize()) before
comparing with sourceDir.toPath(), and keep the existing
runCatching/exception-to-false behavior to treat broken/unresolvable symlinks as
non-opsx symlinks; note generateSkillFiles creates absolute links today but
ensure the new resolution covers relative links too.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c01b9cb-30c0-40f0-8465-8677bc10902c
📒 Files selected for processing (6)
src/main/kotlin/zone/clanker/opsx/Opsx.ktsrc/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.ktsrc/main/kotlin/zone/clanker/opsx/task/SyncTask.ktsrc/test/kotlin/zone/clanker/opsx/OpsxPluginTest.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorAgentTest.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorTest.kt
CLAUDE.md/AGENTS.md now explain what opsx/srcx/wrkx are, how to read srcx context (hub classes, entry points, interfaces, anti-patterns, cross-build), the change lifecycle with status explanations, and the change directory structure. Agent definition (.claude/agents/opsx.md) now includes "Before You Do Anything" section, explains the three integrated systems, and has strict MUST/MUST NOT rules.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/skill/SkillGenerator.kt`:
- Around line 138-148: Before writing the active agent files, remove or unmark
artifacts belonging to any inactive targets so old agent integrations are not
left behind: call activeTarget() to get the current target, iterate over the
list of all known targets (the same source used to choose activeTarget()), and
for each target != activeTarget() delete or clear their instructionFile and
agent file locations (the same paths constructed into agentFile and the
per-target agent/markdown locations you write with writeWithMarkers). Ensure you
remove empty parent directories after deletion and apply the same cleanup logic
to the other write block referenced around lines 151-156 so inactive
.md/.github/ or .agent files are removed or unmarked before writing the active
set.
- Around line 195-221: The generated help text in SkillGenerator (where the
agent prompt is assembled via appendLine calls) uses bare commands like
`opsx-status`, `opsx-propose`, `opsx-apply`, etc., which are incorrect; replace
those literal occurrences with the documented invocation form by reusing the
existing TASK_USAGE constant (or the same source used to build TASK_USAGE) when
assembling the lines (e.g., use TASK_USAGE + " status"/TASK_USAGE + " propose"
or format the command placeholders from TASK_USAGE) so the examples become
`/opsx-*` or `./gradlew -q opsx-*` consistently; update the additional commands
line likewise to reference TASK_USAGE rather than hardcoded names so the docs
never drift.
🪄 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: 913f0a56-cae1-4332-9571-96b41fed8c1d
📒 Files selected for processing (3)
src/main/kotlin/zone/clanker/opsx/skill/SkillGenerator.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorAgentTest.ktsrc/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorTest.kt
✅ Files skipped from review due to trivial changes (1)
- src/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorAgentTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/kotlin/zone/clanker/opsx/skill/SkillGeneratorTest.kt
ProcessBuilder.redirectOutput(appendTo) caused waitFor() to hang because the JVM held the file descriptor open after process exit. Now output is read via a daemon thread that writes to both the log file and Gradle logger, allowing waitFor() to return immediately when the process exits.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/kotlin/zone/clanker/opsx/workflow/AgentDispatcher.kt (1)
50-57: Avoid per-lineappendTextin the hot output loop.Line 55 reopens/writes/closes the file for every line. Under verbose agent output this can become a bottleneck and delay stream consumption.
Refactor to keep a single writer open
- val reader = - Thread { - process.inputStream.bufferedReader().useLines { lines -> - lines.forEach { line -> - logFile.appendText(line + "\n") - logger.quiet(line) - } - } - } + val reader = + Thread { + process.inputStream.bufferedReader().useLines { lines -> + logFile.bufferedWriter().use { writer -> + lines.forEach { line -> + writer.appendLine(line) + logger.quiet(line) + } + } + } + }🤖 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/AgentDispatcher.kt` around lines 50 - 57, The hot loop in AgentDispatcher's reader thread reopens the file per line via logFile.appendText; change the code in the Thread created around process.inputStream.bufferedReader().useLines so it opens a single BufferedWriter (or FileWriter with append=true) before iterating lines, write each line to that writer and call writer.newLine()/flush periodically, and close the writer after the loop; keep logger.quiet(line) per line but remove per-line logFile.appendText calls to avoid repeated open/close overhead.
🤖 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/AgentDispatcher.kt`:
- Line 65: After the bounded join call reader.join(READER_JOIN_TIMEOUT_MS) add
an explicit liveness check: if reader.isAlive() then log a clear error/warn
(include context like "reader thread did not terminate after
READER_JOIN_TIMEOUT_MS"), call reader.interrupt() and attempt a short follow-up
join (e.g., reader.join(READER_JOIN_TIMEOUT_MS / 10)); if still alive, escalate
(log error and/or throw/track for shutdown). Apply the same pattern to the other
join at the same site (the writer thread join around line with writer.join and
its timeout) to avoid leaving dangling threads and truncated logs.
- Around line 63-67: The code currently returns Result(-1, logFile) for both
timeouts and exception fallbacks, conflating distinct failure modes; modify the
Result data class (or create a companion/overload) to include an explicit
failure reason (e.g., a FailureReason enum with values like TIMEOUT,
LAUNCH_ERROR, RUNTIME_ERROR, SUCCESS) and set that field instead of overloading
-1. Update the two return sites in AgentDispatcher's runCatching block (the
timeout branch that calls Result(-1, logFile) and the exception fallback) to
return Result(..., failureReason = FailureReason.TIMEOUT) and Result(...,
failureReason = FailureReason.LAUNCH_ERROR/ RUNTIME_ERROR as appropriate), and
then update callers such as TaskExecutor and VerifyTask to read the new
failureReason instead of interpreting -1.
---
Nitpick comments:
In `@src/main/kotlin/zone/clanker/opsx/workflow/AgentDispatcher.kt`:
- Around line 50-57: The hot loop in AgentDispatcher's reader thread reopens the
file per line via logFile.appendText; change the code in the Thread created
around process.inputStream.bufferedReader().useLines so it opens a single
BufferedWriter (or FileWriter with append=true) before iterating lines, write
each line to that writer and call writer.newLine()/flush periodically, and close
the writer after the loop; keep logger.quiet(line) per line but remove per-line
logFile.appendText calls to avoid repeated open/close overhead.
🪄 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: 349d1b8f-76e5-49b1-ae43-59898501b102
📒 Files selected for processing (1)
src/main/kotlin/zone/clanker/opsx/workflow/AgentDispatcher.kt
- Agent enum (CLAUDE, COPILOT, CODEX, OPENCODE) replaces string-based agent config throughout codebase - SkillGenerator accepts additionalSourceDirs to merge skills from company/project dirs into ~/.clkx/skills/ - AgentDispatcher.dispatch takes Agent enum, not String - TaskExecutor uses Agent enum - All task callers updated to use Agent.fromId()
CleanTask now removes project-level symlinks for ALL agents (not just home dirs), plus agent definition files. README updated with Agent.kt in project structure listing.
- SkillGenerator cleans inactive agent instruction files and definitions before writing the active set (prevents stale artifacts when switching) - AgentDispatcher.Result gains FailureReason enum (TIMEOUT vs DISPATCH_EXCEPTION) - Reader thread gets explicit liveness check + interrupt after join timeout - Agent prompt uses proper /command and ./gradlew forms, not bare names
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
opsx-synconly generates symlinks for the configureddefaultAgent, not all 4 agents.claude/agents/opsx.mdand.github/agents/opsx.mdwith lifecycle knowledgeTest plan
./gradlew buildpassesopsx-syncoutputs "symlinked to claude" (single agent).claude/agents/opsx.mdgenerated with frontmatter and prompt.codex/or.opencode/dirs createdChange proposed via opsx
opsx-propose+opsx-applywith manual task refinement.Summary by CodeRabbit
New Features
Bug Fixes
Tests