fix(approval): resolve preview paths against workspace root, not server CWD (#57)
computeToolPreview/readFileIfExists resolved relative file_write/file_edit paths against the daemon CWD, so the diff shown to the operator for approval read the wrong file (or nothing) when server CWD != workspace_root. Thread the bound workspaceRoot (effectives.policy.workspaceRoot) through and resolve relative paths against it, same as the tools do.
This commit is contained in:
+23
-9
@@ -1275,7 +1275,9 @@ abstract class SessionOrchestrator(
|
|||||||
mode = approvalMode,
|
mode = approvalMode,
|
||||||
)
|
)
|
||||||
val requestId = ApprovalRequestId(UUID.randomUUID().toString())
|
val requestId = ApprovalRequestId(UUID.randomUUID().toString())
|
||||||
val toolPreview = computeToolPreview(toolCall.function.name, parameters)
|
val toolPreview = computeToolPreview(
|
||||||
|
toolCall.function.name, parameters, effectives.policy?.workspaceRoot,
|
||||||
|
)
|
||||||
val domainRequest = DomainApprovalRequest(
|
val domainRequest = DomainApprovalRequest(
|
||||||
id = requestId,
|
id = requestId,
|
||||||
tier = tier,
|
tier = tier,
|
||||||
@@ -3382,10 +3384,14 @@ abstract class SessionOrchestrator(
|
|||||||
* This function performs blocking I/O (file reads) and must be called from a suspend context
|
* This function performs blocking I/O (file reads) and must be called from a suspend context
|
||||||
* that will dispatch it on [Dispatchers.IO].
|
* that will dispatch it on [Dispatchers.IO].
|
||||||
*/
|
*/
|
||||||
private suspend fun computeToolPreview(toolName: String, parameters: Map<String, Any>): String? {
|
private suspend fun computeToolPreview(
|
||||||
|
toolName: String,
|
||||||
|
parameters: Map<String, Any>,
|
||||||
|
workspaceRoot: java.nio.file.Path?,
|
||||||
|
): String? {
|
||||||
if (toolName == "shell") return shellCommandPreview(parameters)
|
if (toolName == "shell") return shellCommandPreview(parameters)
|
||||||
if (toolName == "task_decompose") return renderDecomposePreview(parameters)
|
if (toolName == "task_decompose") return renderDecomposePreview(parameters)
|
||||||
if (toolName == "file_edit") return computeFileEditPreview(parameters)
|
if (toolName == "file_edit") return computeFileEditPreview(parameters, workspaceRoot)
|
||||||
if (toolName != "file_write") return null
|
if (toolName != "file_write") return null
|
||||||
val path = parameters["path"] as? String ?: return null
|
val path = parameters["path"] as? String ?: return null
|
||||||
// file_write no longer carries an `operation` param (delete was split into file_delete), so the
|
// file_write no longer carries an `operation` param (delete was split into file_delete), so the
|
||||||
@@ -3393,18 +3399,23 @@ private suspend fun computeToolPreview(toolName: String, parameters: Map<String,
|
|||||||
// silently bailed to the raw-JSON args fallback for every file_write.
|
// silently bailed to the raw-JSON args fallback for every file_write.
|
||||||
val proposedContent = parameters["content"] as? String ?: return null
|
val proposedContent = parameters["content"] as? String ?: return null
|
||||||
|
|
||||||
val existingContent = readFileIfExists(path)
|
val existingContent = readFileIfExists(path, workspaceRoot)
|
||||||
return buildDiffString(path, existingContent, proposedContent)
|
return buildDiffString(path, existingContent, proposedContent)
|
||||||
}
|
}
|
||||||
|
|
||||||
private suspend fun readFileIfExists(path: String): String? = withContext(Dispatchers.IO) {
|
private suspend fun readFileIfExists(path: String, workspaceRoot: java.nio.file.Path?): String? =
|
||||||
|
withContext(Dispatchers.IO) {
|
||||||
runCatching {
|
runCatching {
|
||||||
val filePath = java.nio.file.Paths.get(path)
|
// Resolve relative paths against the session's workspace root, same as the tools do —
|
||||||
|
// resolving against the daemon CWD showed the operator the wrong file (or nothing) when
|
||||||
|
// server CWD ≠ workspace_root.
|
||||||
|
val raw = java.nio.file.Paths.get(path)
|
||||||
|
val filePath = if (raw.isAbsolute || workspaceRoot == null) raw else workspaceRoot.resolve(raw)
|
||||||
if (java.nio.file.Files.exists(filePath)) {
|
if (java.nio.file.Files.exists(filePath)) {
|
||||||
java.nio.file.Files.readString(filePath)
|
java.nio.file.Files.readString(filePath)
|
||||||
} else null
|
} else null
|
||||||
}.getOrNull()
|
}.getOrNull()
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Preview for `file_edit`'s "replace" and "append" operations: reads the existing file and
|
* Preview for `file_edit`'s "replace" and "append" operations: reads the existing file and
|
||||||
@@ -3412,10 +3423,13 @@ private suspend fun readFileIfExists(path: String): String? = withContext(Dispat
|
|||||||
* to the raw-JSON-args fallback — simulating `patch -p1` application here isn't worth the risk of
|
* to the raw-JSON-args fallback — simulating `patch -p1` application here isn't worth the risk of
|
||||||
* the preview silently disagreeing with what the tool actually does.
|
* the preview silently disagreeing with what the tool actually does.
|
||||||
*/
|
*/
|
||||||
private suspend fun computeFileEditPreview(parameters: Map<String, Any>): String? {
|
private suspend fun computeFileEditPreview(
|
||||||
|
parameters: Map<String, Any>,
|
||||||
|
workspaceRoot: java.nio.file.Path?,
|
||||||
|
): String? {
|
||||||
val path = parameters["path"] as? String ?: return null
|
val path = parameters["path"] as? String ?: return null
|
||||||
val operation = parameters["operation"] as? String ?: return null
|
val operation = parameters["operation"] as? String ?: return null
|
||||||
val existingContent = readFileIfExists(path) ?: return null
|
val existingContent = readFileIfExists(path, workspaceRoot) ?: return null
|
||||||
|
|
||||||
val proposedContent = when (operation) {
|
val proposedContent = when (operation) {
|
||||||
"append" -> existingContent + (parameters["content"] as? String ?: return null)
|
"append" -> existingContent + (parameters["content"] as? String ?: return null)
|
||||||
|
|||||||
Reference in New Issue
Block a user