Skip to content

Compiling and tests passing backend-shared#45

Merged
amichne merged 3 commits intomainfrom
feature/intellij-plugin-piggyback
Apr 9, 2026
Merged

Compiling and tests passing backend-shared#45
amichne merged 3 commits intomainfrom
feature/intellij-plugin-piggyback

Conversation

@amichne
Copy link
Copy Markdown
Owner

@amichne amichne commented Apr 9, 2026

No description provided.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 6 additional findings in Devin Review.

Open in Devin Review

limits = limits,
)

val socketPath = defaultSocketPath(workspaceRoot)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Standalone and IntelliJ backends use the same socket path, causing socket hijacking

Both KastPluginService.startServer() and the standalone backend resolve socket paths via defaultSocketPath(workspaceRoot), which returns the same single path (<workspace>/.kast/s). When the second backend starts, UnixDomainSocketRpcServer.start() at analysis-server/src/main/kotlin/io/github/amichne/kast/server/LocalRpcServer.kt:47 calls socketPath.deleteIfExists() before binding — silently deleting the first backend's active socket and taking it over. This breaks the first backend's connections and makes the two backends unable to coexist for the same workspace, directly contradicting the PR's design intent (CLI prefers IntelliJ when both are available per WorkspaceRuntimeManager.kt:382-386, tests at WorkspaceRuntimeManagerTest.kt:292 verify both backends discovered simultaneously, and docs describe both running side-by-side).

Prompt for agents
The IntelliJ plugin backend and the standalone backend both call defaultSocketPath(workspaceRoot) which returns the same path (<workspace>/.kast/s). Since UnixDomainSocketRpcServer.start() deletes the existing socket file before binding, whichever backend starts second will silently destroy the first backend's socket.

To fix this, the two backends need distinct socket paths. The simplest approach is to make defaultSocketPath backend-aware, or have the IntelliJ plugin use a different socket name. For example:

- In KastPluginService.startServer(), compute a distinct socket path like workspaceMetadataDirectory(workspaceRoot).resolve("s-intellij") instead of calling defaultSocketPath(workspaceRoot).
- Alternatively, add a backendName parameter to defaultSocketPath() in analysis-api/src/main/kotlin/io/github/amichne/kast/api/WorkspacePaths.kt so that each backend gets its own socket (e.g. <workspace>/.kast/s for standalone, <workspace>/.kast/s-intellij for the plugin).

Either way, the descriptor written by AnalysisServer.start() already includes the socketPath, so the CLI will discover the correct socket for each backend regardless of the naming convention.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +110 to +111
val listA = if (unorderedArrayKeys.isEmpty()) a.toList() else a.sortedBy { it.toString() }
val listB = if (unorderedArrayKeys.isEmpty()) b.toList() else b.sortedBy { it.toString() }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Structural comparator sorts ALL arrays when any unorderedArrayKeys are specified

ParityComparator.Structural.compareElements checks unorderedArrayKeys.isEmpty() globally rather than checking whether the current array's parent key is in unorderedArrayKeys. When a caller passes e.g. unorderedArrayKeys = setOf("references"), every JsonArray encountered anywhere in the tree is sorted — not just the ones under the "references" key. This could cause parity tests to incorrectly pass when two backends return the same elements in different order for arrays that should be order-sensitive. Contrast with the Unordered comparator at line 66 which correctly checks key in unorderedArrayKeys per field.

Prompt for agents
In ParityComparator.Structural.compareElements, the JsonArray branch at lines 110-111 uses a global check (unorderedArrayKeys.isEmpty()) to decide whether to sort arrays. This means ALL arrays are sorted when any unordered key is specified, not just the arrays at the specified keys.

To fix this, the compareElements function needs to know the current key context when processing arrays. One approach: thread the parent key name through the recursive calls. In the JsonObject branch, when recursing into a value whose key is in unorderedArrayKeys and the value is a JsonArray, sort that array; otherwise preserve order. This mirrors the approach already used in ParityComparator.Unordered at line 66.

Relevant files:
- parity-tests/src/test/kotlin/io/github/amichne/kast/parity/ParityComparator.kt (Structural class, compareElements method)
- The Unordered class in the same file shows the correct pattern at lines 59-77
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@amichne amichne merged commit 2cfaac5 into main Apr 9, 2026
4 checks passed
@amichne amichne deleted the feature/intellij-plugin-piggyback branch April 9, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant