From 07d5c1db526d9fe880147f20427feb8f9881ff7b Mon Sep 17 00:00:00 2001 From: slhafzjw Date: Tue, 24 Feb 2026 23:55:31 +0800 Subject: [PATCH] feat: require explicit @param required flag and improve run failure output with params details --- .../kotlin/work/slhaf/hub/ScriptEngine.kt | 36 ++++++++++-- .../kotlin/work/slhaf/hub/WebScriptService.kt | 38 ++++++++++++- .../kotlin/work/slhaf/hub/WebHostApiTest.kt | 57 +++++++++++++++++++ tools/slhaf-hub-tui.kts | 25 +++++++- 4 files changed, 146 insertions(+), 10 deletions(-) diff --git a/src/main/kotlin/work/slhaf/hub/ScriptEngine.kt b/src/main/kotlin/work/slhaf/hub/ScriptEngine.kt index 5df5cad..a7b6ce1 100644 --- a/src/main/kotlin/work/slhaf/hub/ScriptEngine.kt +++ b/src/main/kotlin/work/slhaf/hub/ScriptEngine.kt @@ -208,10 +208,14 @@ fun validateScriptMetadata(scriptContent: String): List { errors += "line $lineNo: duplicate @param name '$name'. param names must be unique." } + var hasRequiredOption = false parts.drop(1).forEach { part -> when { - part.equals("required", ignoreCase = true) -> Unit + part.equals("required", ignoreCase = true) -> { + hasRequiredOption = true + } part.startsWith("required=", ignoreCase = true) -> { + hasRequiredOption = true val v = part.substringAfter("=").trim() if (!v.equals("true", ignoreCase = true) && !v.equals("false", ignoreCase = true)) { errors += "line $lineNo: invalid required value '$v'. expected true/false." @@ -224,6 +228,9 @@ fun validateScriptMetadata(scriptContent: String): List { } } } + if (!hasRequiredOption) { + errors += "line $lineNo: missing required option. expected '@param: name | required=true|false | default=value | desc=text'." + } } } @@ -282,6 +289,14 @@ fun loadMetadataFromComments(scriptFile: File): ScriptMetadata { } fun evalAndCapture(scriptFile: File, requestContext: ScriptRequestContext = ScriptRequestContext()): ScriptExecutionResult { + return evalAndCapture(scriptFile, requestContext, enforceRequiredParams = true) +} + +fun evalAndCapture( + scriptFile: File, + requestContext: ScriptRequestContext = ScriptRequestContext(), + enforceRequiredParams: Boolean, +): ScriptExecutionResult { synchronized(evalLock) { val oldOut = System.out val oldErr = System.err @@ -298,30 +313,40 @@ fun evalAndCapture(scriptFile: File, requestContext: ScriptRequestContext = Scri .filter { p -> p.required && requestContext.args.none { token -> token.substringBefore("=", missingDelimiterValue = "") == p.name - } && p.defaultValue == null + } } .map { it.name } val injected = injectArgsDeclaration(original, requestContext.args) val result = evalSource(injected.toScriptSource(scriptFile.name)) + val returnValueError = (result as? ResultWithDiagnostics.Success) + ?.value + ?.returnValue as? ResultValue.Error + val hasErrorDiagnostics = result.reports.any { + it.severity == ScriptDiagnostic.Severity.ERROR || it.severity == ScriptDiagnostic.Severity.FATAL + } val diagnostics = result.reports .filter { it.severity > ScriptDiagnostic.Severity.DEBUG } .joinToString("\n") { val ex = it.exception?.let { e -> ": ${e::class.simpleName}: ${e.message}" } ?: "" "[${it.severity}] ${it.message}$ex" } - val missingMessage = if (missingRequired.isEmpty()) "" else + val missingMessage = if (!enforceRequiredParams || missingRequired.isEmpty()) "" else "[ERROR] Missing required parameters: ${missingRequired.joinToString(", ")}" val output = buffer.toString(Charsets.UTF_8.name()).trim() val finalText = buildString { if (output.isNotEmpty()) appendLine(output) + if (returnValueError != null) appendLine("[ERROR] ${returnValueError.error::class.simpleName}: ${returnValueError.error.message}") if (diagnostics.isNotEmpty()) appendLine(diagnostics) if (missingMessage.isNotEmpty()) appendLine(missingMessage) }.trim() ScriptExecutionResult( - ok = result is ResultWithDiagnostics.Success && missingRequired.isEmpty(), + ok = result is ResultWithDiagnostics.Success && + !hasErrorDiagnostics && + returnValueError == null && + (!enforceRequiredParams || missingRequired.isEmpty()), output = finalText, metadata = metadata, missingRequiredParams = missingRequired, @@ -340,10 +365,11 @@ fun evalAndCaptureWithTimeout( scriptFile: File, requestContext: ScriptRequestContext = ScriptRequestContext(), timeoutMs: Long, + enforceRequiredParams: Boolean = true, ): ScriptExecutionResult { val boundedTimeout = timeoutMs.coerceAtLeast(1) val future = evalExecutor.submit { - evalAndCapture(scriptFile, requestContext) + evalAndCapture(scriptFile, requestContext, enforceRequiredParams = enforceRequiredParams) } return try { future.get(boundedTimeout, TimeUnit.MILLISECONDS) diff --git a/src/main/kotlin/work/slhaf/hub/WebScriptService.kt b/src/main/kotlin/work/slhaf/hub/WebScriptService.kt index 911fc72..b5526a8 100644 --- a/src/main/kotlin/work/slhaf/hub/WebScriptService.kt +++ b/src/main/kotlin/work/slhaf/hub/WebScriptService.kt @@ -62,6 +62,34 @@ private fun metadataValidationMessage(errors: List): String = appendLine("// @param: name | required=false | default=world | desc=Name to greet") }.trim() +private fun formatParamsHint(metadata: ScriptMetadata): String = + buildString { + appendLine("params:") + if (metadata.params.isEmpty()) { + appendLine("- (none)") + return@buildString + } + metadata.params.forEach { p -> + val defaultPart = p.defaultValue?.let { ", default=$it" } ?: "" + val descPart = p.description?.let { ", desc=$it" } ?: "" + appendLine("- ${p.name} (required=${p.required}$defaultPart$descPart)") + } + }.trimEnd() + +private fun runFailureMessage(result: ScriptExecutionResult): String { + val title = when { + result.timedOut -> "[ERROR] script execution timed out" + result.missingRequiredParams.isNotEmpty() -> + "[ERROR] script execution failed: missing required params: ${result.missingRequiredParams.joinToString(", ")}" + else -> "[ERROR] script execution failed" + } + return buildString { + appendLine(title) + if (result.output.isNotBlank()) appendLine(result.output) + append(formatParamsHint(result.metadata)) + }.trim() +} + fun metadataJson(scriptName: String, metadata: ScriptMetadata, source: String): String { val description = metadata.description?.let { "\"${it.jsonEscaped()}\"" } ?: "null" val params = metadata.params.joinToString(",") { param -> @@ -105,7 +133,7 @@ suspend fun handleCreateScript(call: ApplicationCall, scriptsDir: File) { script.writeText(content) removeCachedMetadata(script) - val result = evalAndCapture(script, ScriptRequestContext()) + val result = evalAndCapture(script, ScriptRequestContext(), enforceRequiredParams = false) if (!result.ok) { script.delete() removeCachedMetadata(script) @@ -169,7 +197,7 @@ suspend fun handleUpdateScript(call: ApplicationCall, scriptsDir: File) { script.writeText(newContent) removeCachedMetadata(script) - val result = evalAndCapture(script, ScriptRequestContext()) + val result = evalAndCapture(script, ScriptRequestContext(), enforceRequiredParams = false) if (!result.ok) { script.writeText(previousContent) removeCachedMetadata(script) @@ -221,7 +249,11 @@ suspend fun handleRunRequest(call: ApplicationCall, scriptsDir: File, consumeBod } call.respondText( - result.output.ifBlank { if (result.ok) "OK" else "FAILED" }, + if (result.ok) { + result.output.ifBlank { "OK" } + } else { + runFailureMessage(result) + }, status = status, contentType = ContentType.Text.Plain ) diff --git a/src/test/kotlin/work/slhaf/hub/WebHostApiTest.kt b/src/test/kotlin/work/slhaf/hub/WebHostApiTest.kt index 122f2cb..ed9e273 100644 --- a/src/test/kotlin/work/slhaf/hub/WebHostApiTest.kt +++ b/src/test/kotlin/work/slhaf/hub/WebHostApiTest.kt @@ -182,6 +182,63 @@ class WebHostApiTest { assertTrue(run.bodyAsText().contains("timed out")) } + @Test + fun metadataRequiresExplicitRequiredField() = withApp { _ -> + val create = client.post("/scripts/badmeta") { + bearerRoot() + setBody( + """ + // @desc: bad metadata + // @param: name | default=world | desc=missing required + val args: Array = emptyArray() + println("ok") + """.trimIndent() + ) + } + assertEquals(HttpStatusCode.BadRequest, create.status) + val body = create.bodyAsText() + assertTrue(body.contains("metadata validation failed")) + assertTrue(body.contains("missing required option")) + } + + @Test + fun runErrorResponseIncludesParamsAndRequiredCheck() = withApp { _ -> + val create = client.post("/scripts/runner") { + bearerRoot() + setBody( + """ + // @desc: run test + // @param: must | required=true | default=world | desc=Must be provided explicitly + // @param: boom | required=false | default=false | desc=Trigger runtime failure + val args: Array = emptyArray() + val kv = args.mapNotNull { + val i = it.indexOf('=') + if (i <= 0) null else it.substring(0, i) to it.substring(i + 1) + }.toMap() + if ((kv["boom"] ?: "false").equals("true", ignoreCase = true)) { + error("boom") + } + println("must=" + (kv["must"] ?: "none")) + """.trimIndent() + ) + } + assertEquals(HttpStatusCode.Created, create.status) + + val missingRequired = client.get("/run/runner") { bearerRoot() } + assertEquals(HttpStatusCode.BadRequest, missingRequired.status) + val missingBody = missingRequired.bodyAsText() + assertTrue(missingBody.contains("missing required params: must")) + assertTrue(missingBody.contains("params:")) + assertTrue(missingBody.contains("must (required=true")) + + val runtimeError = client.get("/run/runner?must=ok&boom=true") { bearerRoot() } + assertEquals(HttpStatusCode.InternalServerError, runtimeError.status) + val runtimeErrorBody = runtimeError.bodyAsText() + assertTrue(runtimeErrorBody.contains("script execution failed")) + assertTrue(runtimeErrorBody.contains("params:")) + assertTrue(runtimeErrorBody.contains("boom (required=false")) + } + private fun withApp(testBlock: suspend io.ktor.server.testing.ApplicationTestBuilder.(java.nio.file.Path) -> Unit) { val scriptsDir = createTempDirectory("webhost-api-test-") tempDirs.add(scriptsDir) diff --git a/tools/slhaf-hub-tui.kts b/tools/slhaf-hub-tui.kts index 9a9be91..b9bdbfb 100755 --- a/tools/slhaf-hub-tui.kts +++ b/tools/slhaf-hub-tui.kts @@ -225,10 +225,31 @@ fun colorizeStatusLine(line: String): String = fun prettyPrintJsonOrKeep(raw: String): String { val text = raw.trim() - if (!(text.startsWith("{") || text.startsWith("["))) return raw + if (!isLikelyJson(text)) return raw return runCatching { prettyJson(text) }.getOrElse { raw } } +fun isLikelyJson(text: String): Boolean { + if (text.isBlank()) return false + val first = text.first() + if (first != '{' && first != '[') return false + + val second = text.drop(1).firstOrNull { !it.isWhitespace() } ?: return false + return when (first) { + '{' -> second == '"' || second == '}' + '[' -> second == ']' || + second == '{' || + second == '[' || + second == '"' || + second == '-' || + second.isDigit() || + second == 't' || + second == 'f' || + second == 'n' + else -> false + } +} + fun prettyJson(input: String): String { val sb = StringBuilder(input.length + 32) var indent = 0 @@ -455,7 +476,7 @@ fun initialScriptTemplate(name: String): String = """ // @desc: $name // @timeout: 10s -// @param: sample | default=value | desc=example parameter +// @param: sample | required=false | default=value | desc=example parameter val args: Array = emptyArray() val kv = args.mapNotNull {