From 1786a165234ae1e1d843f1078d09ae68454e3c68 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Tue, 11 Aug 2026 17:41:03 -0700 Subject: [PATCH 1/2] feat(studio-bridge): carry script return values back over the bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An executed script's return value had nowhere to go: the plugin discarded everything pcall handed back, and a caller could only learn what a run produced by reading its printed output. Anything a caller must read back exactly — test counts, for instance — had to be scraped out of log text. The plugin's execute action now marshals every returned value onto scriptComplete's payload.returnValues, and it surfaces on both ExecResult and StudioBridgeResult. Tables are walked recursively (array-like tables stay arrays) and Roblox datatypes reuse the SerializedValue shapes. The field stays absent whenever no scriptComplete arrived, which keeps "we never learned what it returned" distinct from "it returned nothing". Marshalling walks values the script chose, so it is written to survive hostile ones. Traversal is raw — next, rawlen, rawget — so __iter, __len and __index can neither misreport a table's contents nor throw from inside the walk. Cycles, nesting past 64 levels, non-finite numbers and keys that collapse onto one JSON key are marked rather than producing JSON the server cannot decode, and a node budget bounds total work: the cycle set is cleared as the walk leaves a table, so 30 levels of shared references would otherwise expand into 2^30 nodes. Whatever still throws degrades the whole result to one marker. That last guard matters beyond the value itself. Marshalling runs inside _processQueue's window, and a throw there left _processing latched for the rest of the Studio session — every later execute request piled up undrained, with no self-healing, since syncActions sees an unchanged hash and never re-registers the action. Each queued request is now run under pcall and answered with an INTERNAL_ERROR if it throws, so the queue is released either way. That hole predates return values (an unguarded JSONEncode in sendMessage could already reach it) but marshalling user data is what made it easy to hit. Nothing consumes the new field yet. --- .../src/bridge/bridge-session.test.ts | 45 +++ .../src/bridge/bridge-session.ts | 1 + tools/studio-bridge/src/bridge/types.ts | 15 +- .../src/commands/console/exec/execute.luau | 196 ++++++++++++- tools/studio-bridge/src/index.ts | 1 + .../src/server/studio-bridge-server.test.ts | 49 ++++ .../src/server/studio-bridge-server.ts | 10 + .../src/server/web-socket-protocol.test.ts | 64 +++++ .../src/server/web-socket-protocol.ts | 29 ++ .../test/execute-handler.test.luau | 259 ++++++++++++++++++ 10 files changed, 664 insertions(+), 5 deletions(-) diff --git a/tools/studio-bridge/src/bridge/bridge-session.test.ts b/tools/studio-bridge/src/bridge/bridge-session.test.ts index b93f39ec6ca..f7afd7bc447 100644 --- a/tools/studio-bridge/src/bridge/bridge-session.test.ts +++ b/tools/studio-bridge/src/bridge/bridge-session.test.ts @@ -174,6 +174,51 @@ describe('BridgeSession', () => { expect(result.success).toBe(false); expect(result.error).toBe('boom'); }); + + it('surfaces the values the script returned', async () => { + const handle = new MockTransportHandle(); + handle.sendActionAsync.mockResolvedValueOnce({ + type: 'scriptComplete', + sessionId: 'session-1', + payload: { + success: true, + returnValues: [{ counts: { passed: 3 } }, 42], + }, + }); + + const session = new BridgeSession(createSessionInfo(), handle); + const result = await session.execAsync('return results, 42'); + + expect(result.returnValues).toEqual([{ counts: { passed: 3 } }, 42]); + }); + + it('leaves returnValues undefined when the plugin reported none', async () => { + const handle = new MockTransportHandle(); + handle.sendActionAsync.mockResolvedValueOnce({ + type: 'scriptComplete', + sessionId: 'session-1', + payload: { success: true }, + }); + + const session = new BridgeSession(createSessionInfo(), handle); + const result = await session.execAsync('print("hello")'); + + expect(result.returnValues).toBeUndefined(); + }); + + it('leaves returnValues undefined on an error response', async () => { + const handle = new MockTransportHandle(); + handle.sendActionAsync.mockResolvedValueOnce({ + type: 'error', + sessionId: 'session-1', + payload: { code: 'SCRIPT_RUNTIME_ERROR', message: 'boom' }, + }); + + const session = new BridgeSession(createSessionInfo(), handle); + const result = await session.execAsync('error("boom")'); + + expect(result.returnValues).toBeUndefined(); + }); }); describe('queryStateAsync', () => { diff --git a/tools/studio-bridge/src/bridge/bridge-session.ts b/tools/studio-bridge/src/bridge/bridge-session.ts index f26a3cbdffe..c488ed51e3d 100644 --- a/tools/studio-bridge/src/bridge/bridge-session.ts +++ b/tools/studio-bridge/src/bridge/bridge-session.ts @@ -118,6 +118,7 @@ export class BridgeSession extends EventEmitter { success: result.payload.success, output, error: result.payload.error, + returnValues: result.payload.returnValues, }; } diff --git a/tools/studio-bridge/src/bridge/types.ts b/tools/studio-bridge/src/bridge/types.ts index 200ccfc6cd5..6480f29b905 100644 --- a/tools/studio-bridge/src/bridge/types.ts +++ b/tools/studio-bridge/src/bridge/types.ts @@ -11,10 +11,17 @@ import type { Capability, DataModelInstance, OutputLevel, + SerializedReturnValue, } from '../server/web-socket-protocol.js'; // Re-export protocol types used in the public API -export type { StudioState, Capability, DataModelInstance, OutputLevel }; +export type { + StudioState, + Capability, + DataModelInstance, + OutputLevel, + SerializedReturnValue, +}; export type SessionContext = 'edit' | 'client' | 'server'; export type SessionOrigin = 'user' | 'managed'; @@ -47,6 +54,12 @@ export interface ExecResult { success: boolean; output: Array<{ level: OutputLevel; body: string }>; error?: string; + /** + * Everything the script returned, in order. Absent when the plugin reported + * no return channel at all (older plugin build, or a run that never reached + * a return) — distinct from `[]`, which means it returned nothing. + */ + returnValues?: SerializedReturnValue[]; } export interface StateResult { diff --git a/tools/studio-bridge/src/commands/console/exec/execute.luau b/tools/studio-bridge/src/commands/console/exec/execute.luau index 734a588c2b9..f023dbaae73 100644 --- a/tools/studio-bridge/src/commands/console/exec/execute.luau +++ b/tools/studio-bridge/src/commands/console/exec/execute.luau @@ -10,6 +10,12 @@ - Distinct error codes: SCRIPT_LOAD_ERROR, SCRIPT_RUNTIME_ERROR. - Sequential queueing: concurrent execute requests are processed one at a time in FIFO order. + - Return values: everything the script returns is marshalled onto + `payload.returnValues`, so a caller can read a result as a value + instead of scraping it back out of printed output. + - Failure isolation: a request that throws while marshalling or + reporting its result is answered with an error rather than left to + latch the queue for the rest of the session. This module has no Roblox dependencies and is testable under Lune. ]] @@ -24,6 +30,149 @@ local _queue: { { payload: { [string]: any }, requestId: string?, sessionId: str {} local _processing = false +-- --------------------------------------------------------------------------- +-- Return value serialization +-- --------------------------------------------------------------------------- + +-- Produces the SerializedReturnValue shapes declared in +-- web-socket-protocol.ts. Action modules are loadstring'd standalone inside +-- the plugin and cannot require anything, so this cannot be shared with the +-- equivalent marshaller in query-data-model.luau. + +local MAX_DEPTH = 64 + +-- The depth cap alone does not bound the work. `seen` is cleared as the walk +-- leaves a table -- correct, since a value reachable by two separate paths is +-- not a cycle -- which means a shared reference is expanded once per edge. +-- `for i = 1, 30 do x = { a = x, b = x } end` is only 30 levels deep but 2^30 +-- nodes, so a total node budget is what actually keeps Studio alive. +local MAX_NODES = 100_000 + +type MarshalState = { seen: { [any]: boolean }, budget: number } + +local function unsupported(typeName: string, text: string): { [string]: any } + return { type = "Unsupported", typeName = typeName, toString = text } +end + +local serializeValue: (value: any, depth: number, state: MarshalState) -> any + +-- An array-like table becomes a JSON array, anything else a JSON object with +-- string keys. Mixed tables take the object branch on purpose: JSONEncode +-- rejects them, and a rejected encode drops the whole scriptComplete message. +-- +-- Traversal is raw (`next`, `rawlen`, `rawget`) so `__iter`, `__len` and +-- `__index` cannot make a returned table report contents it does not have -- +-- and cannot throw from inside the walk. A metatable is not part of the value, +-- so what gets marshalled is the table as it actually is. +local function serializeTable(value: { [any]: any }, depth: number, state: MarshalState): any + local count = 0 + for _ in next, value do + count += 1 + end + + if count == rawlen(value) then + local array = {} + for index = 1, count do + array[index] = serializeValue(rawget(value :: any, index), depth + 1, state) + end + return array + end + + local map: { [string]: any } = {} + for key, item in next, value do + local stringKey = if type(key) == "string" then key else tostring(key) + if map[stringKey] ~= nil then + -- Two distinct Luau keys collapsed onto one JSON key (`[1]` and + -- `"1"`). Marking it keeps the loss visible rather than quietly + -- serving whichever one the traversal happened to reach last. + map[stringKey] = unsupported("collision", ``) + else + map[stringKey] = serializeValue(item, depth + 1, state) + end + end + return map +end + +function serializeValue(value: any, depth: number, state: MarshalState): any + local valueType = typeof(value) + + state.budget -= 1 + if state.budget < 0 then + return unsupported(valueType, "") + end + + if valueType == "nil" then + -- A JSON array cannot hold a hole, and dropping the entry would shift + -- every later value into the wrong position. + return { type = "Nil" } + elseif valueType == "string" or valueType == "boolean" then + return value + elseif valueType == "number" then + -- inf and nan have no JSON spelling, and encoding one produces a + -- document the server cannot decode -- losing the entire message. + if value ~= value or value == math.huge or value == -math.huge then + return unsupported("number", tostring(value)) + end + return value + elseif valueType == "table" then + if depth > MAX_DEPTH then + return unsupported("table", "") + end + if state.seen[value] then + return unsupported("table", "") + end + state.seen[value] = true + local serialized = serializeTable(value, depth, state) + state.seen[value] = nil + return serialized + elseif valueType == "Vector3" then + return { type = "Vector3", value = { value.X, value.Y, value.Z } } + elseif valueType == "Vector2" then + return { type = "Vector2", value = { value.X, value.Y } } + elseif valueType == "CFrame" then + return { type = "CFrame", value = { value:GetComponents() } } + elseif valueType == "Color3" then + return { type = "Color3", value = { value.R, value.G, value.B } } + elseif valueType == "UDim2" then + return { type = "UDim2", value = { value.X.Scale, value.X.Offset, value.Y.Scale, value.Y.Offset } } + elseif valueType == "UDim" then + return { type = "UDim", value = { value.Scale, value.Offset } } + elseif valueType == "BrickColor" then + return { type = "BrickColor", name = value.Name, value = value.Number } + elseif valueType == "EnumItem" then + return { type = "EnumItem", enum = tostring(value.EnumType), name = value.Name, value = value.Value } + elseif valueType == "Instance" then + return { type = "Instance", className = value.ClassName, path = value:GetFullName() } + else + return unsupported(valueType, tostring(value)) + end +end + +-- Marshal the values a script returned, in order. The depth cap, cycle set and +-- node budget are shared across all of them, so one run cannot be talked into +-- unbounded work by spreading it over several return values. +local function serializeReturnValues(packed: { n: number, [number]: any }): { any } + local state: MarshalState = { seen = {}, budget = MAX_NODES } + local returnValues = {} + for index = 2, packed.n do + returnValues[index - 1] = serializeValue(packed[index], 1, state) + end + return returnValues +end + +-- Marshalling walks values the script chose, so a hostile metatable can still +-- throw from somewhere the raw traversal cannot avoid -- a table key whose +-- `__tostring` errors, say. Degrading to a marker keeps one bad value from +-- failing the run, and keeps it from throwing out of _handleExecute, which in +-- queue mode would latch _processing and strand every later request. +local function trySerializeReturnValues(packed: { n: number, [number]: any }): { any } + local ok, returnValues = pcall(serializeReturnValues, packed) + if ok then + return returnValues + end + return { unsupported("unknown", ``) } +end + -- --------------------------------------------------------------------------- -- Core execution logic -- --------------------------------------------------------------------------- @@ -108,17 +257,27 @@ function ExecuteAction._handleExecute( originalWarn(...) end - local success, runtimeError = pcall(fn) + local returned = table.pack(pcall(fn)) + local success = returned[1] -- Restore originals env.print = originalPrint env.warn = originalWarn if not success then - return sendResult({ success = false, error = tostring(runtimeError), code = "SCRIPT_RUNTIME_ERROR", output = captured }) + return sendResult({ + success = false, + error = tostring(returned[2]), + code = "SCRIPT_RUNTIME_ERROR", + output = captured, + }) end - return sendResult({ success = true, output = captured }) + return sendResult({ + success = true, + output = captured, + returnValues = trySerializeReturnValues(returned), + }) end -- --------------------------------------------------------------------------- @@ -133,7 +292,36 @@ local function _processQueue() while #_queue > 0 do local item = table.remove(_queue, 1) - ExecuteAction._handleExecute(item.payload, item.requestId, item.sessionId, item.sendMessage) + -- A throw here must not escape the loop. _processing would stay latched + -- for the rest of the Studio session, every later execute request would + -- pile up in _queue with nothing left to drain it, and the plugin never + -- re-registers this action on its own (syncActions sees an unchanged + -- hash), so only restarting Studio would recover. The most likely + -- thrower is sendMessage itself: it JSONEncodes a payload built partly + -- out of values the script chose. + local ok, err = + pcall(ExecuteAction._handleExecute, item.payload, item.requestId, item.sessionId, item.sendMessage) + + if not ok and item.sendMessage then + -- Tell the requester, or the CLI waits out its whole timeout with no + -- idea the request was dropped. Small fixed payload, so the encode + -- that just failed cannot fail again for the same reason -- but it + -- is still guarded, since it is the last thing standing between a + -- bad response and the latch above. + local response: { [string]: any } = { + type = "scriptComplete", + sessionId = item.sessionId, + payload = { + success = false, + error = `Failed to report script result: {tostring(err)}`, + code = "INTERNAL_ERROR", + }, + } + if item.requestId ~= nil and item.requestId ~= "" then + response.requestId = item.requestId + end + pcall(item.sendMessage, response) + end end _processing = false diff --git a/tools/studio-bridge/src/index.ts b/tools/studio-bridge/src/index.ts index dcde2a4af88..7408adf1852 100644 --- a/tools/studio-bridge/src/index.ts +++ b/tools/studio-bridge/src/index.ts @@ -55,6 +55,7 @@ export type { DataModelInstance, ErrorCode, SerializedValue, + SerializedReturnValue, } from './server/web-socket-protocol.js'; // Lower-level exports for advanced usage / testing diff --git a/tools/studio-bridge/src/server/studio-bridge-server.test.ts b/tools/studio-bridge/src/server/studio-bridge-server.test.ts index 61064734284..f4e6d16c57f 100644 --- a/tools/studio-bridge/src/server/studio-bridge-server.test.ts +++ b/tools/studio-bridge/src/server/studio-bridge-server.test.ts @@ -285,6 +285,55 @@ describe('StudioBridgeServer', () => { expect(result.logs).toContain('Script threw: boom'); }); + it('carries the script return values off the scriptComplete', async () => { + const ready = await createReadyServer(); + server = ready.server; + client = ready.client; + + const resultPromise = server.executeAsync({ + scriptContent: 'return { counts = { passed = 7 } }', + }); + + await new Promise((resolve) => { + client!.on('message', (raw) => { + const data = JSON.parse( + typeof raw === 'string' ? raw : raw.toString('utf-8') + ); + if (data.type === 'execute') resolve(); + }); + }); + + client!.send( + JSON.stringify({ + type: 'scriptComplete', + sessionId: ready.sessionId, + payload: { + success: true, + returnValues: [{ counts: { passed: 7 } }], + }, + }) + ); + + const result = await resultPromise; + expect(result.returnValues).toEqual([{ counts: { passed: 7 } }]); + }); + + it('leaves returnValues undefined when the run times out', async () => { + // Nothing reported a return channel, so the value is unknown rather than + // empty — a caller can still fall back to whatever output arrived. + const ready = await createReadyServer(); + server = ready.server; + client = ready.client; + + const result = await server.executeAsync({ + scriptContent: 'while true do end', + timeoutMs: 200, + }); + + expect(result.success).toBe(false); + expect(result.returnValues).toBeUndefined(); + }); + it('returns failure when client disconnects during execution', async () => { const ready = await createReadyServer(); server = ready.server; diff --git a/tools/studio-bridge/src/server/studio-bridge-server.ts b/tools/studio-bridge/src/server/studio-bridge-server.ts index 814b37a6e60..1e1824cec50 100644 --- a/tools/studio-bridge/src/server/studio-bridge-server.ts +++ b/tools/studio-bridge/src/server/studio-bridge-server.ts @@ -26,6 +26,7 @@ import { type Capability, type PluginMessage, type ServerMessage, + type SerializedReturnValue, encodeMessage, decodePluginMessage, } from './web-socket-protocol.js'; @@ -106,6 +107,14 @@ export interface ExecuteOptions { export interface StudioBridgeResult { success: boolean; logs: string; + /** + * Everything the script returned, in order, marshalled by the plugin. + * Absent whenever no `scriptComplete` carrying a return channel arrived — + * a timeout, a disconnect, a plugin error, or an older plugin build — which + * is what separates "we never learned what it returned" from `[]`, "it + * returned nothing". + */ + returnValues?: SerializedReturnValue[]; } type BridgeState = @@ -797,6 +806,7 @@ export class StudioBridgeServer { finish({ success: msg.payload.success, logs: logLines.join('\n'), + returnValues: msg.payload.returnValues, }); break; } diff --git a/tools/studio-bridge/src/server/web-socket-protocol.test.ts b/tools/studio-bridge/src/server/web-socket-protocol.test.ts index 853cf4e8016..21bda5b0ebe 100644 --- a/tools/studio-bridge/src/server/web-socket-protocol.test.ts +++ b/tools/studio-bridge/src/server/web-socket-protocol.test.ts @@ -47,6 +47,70 @@ describe('decodePluginMessage', () => { }); expect(msg).not.toHaveProperty('requestId'); }); + + it('carries returnValues through unchanged, nesting included', () => { + const msg = roundTripPlugin({ + type: 'scriptComplete', + sessionId: 'sess-1', + payload: { + success: true, + returnValues: [ + { slug: 'maid', counts: { passed: 1014, failed: 0 } }, + 'str', + 42, + true, + { type: 'Vector3', value: [1, 2, 3] }, + [1, 2], + ], + }, + }); + expect(msg?.type).toBe('scriptComplete'); + expect( + (msg as { payload: { returnValues?: unknown[] } }).payload.returnValues + ).toEqual([ + { slug: 'maid', counts: { passed: 1014, failed: 0 } }, + 'str', + 42, + true, + { type: 'Vector3', value: [1, 2, 3] }, + [1, 2], + ]); + }); + + it('leaves returnValues undefined when the plugin sent none', () => { + // Distinguishable from an empty array: nothing reported a return channel, + // so a caller can still fall back to the run's printed output. + const msg = roundTripPlugin({ + type: 'scriptComplete', + sessionId: 'sess-1', + payload: { success: true }, + }); + expect( + (msg as { payload: { returnValues?: unknown[] } }).payload.returnValues + ).toBeUndefined(); + }); + + it('preserves an empty returnValues array', () => { + const msg = roundTripPlugin({ + type: 'scriptComplete', + sessionId: 'sess-1', + payload: { success: true, returnValues: [] }, + }); + expect( + (msg as { payload: { returnValues?: unknown[] } }).payload.returnValues + ).toEqual([]); + }); + + it('ignores a returnValues field that is not an array', () => { + const msg = roundTripPlugin({ + type: 'scriptComplete', + sessionId: 'sess-1', + payload: { success: true, returnValues: 'nope' }, + }); + expect( + (msg as { payload: { returnValues?: unknown[] } }).payload.returnValues + ).toBeUndefined(); + }); }); describe('register', () => { diff --git a/tools/studio-bridge/src/server/web-socket-protocol.ts b/tools/studio-bridge/src/server/web-socket-protocol.ts index 99eb17a2fe0..4587b34e3da 100644 --- a/tools/studio-bridge/src/server/web-socket-protocol.ts +++ b/tools/studio-bridge/src/server/web-socket-protocol.ts @@ -69,6 +69,22 @@ export type SerializedValue = | { type: 'Instance'; className: string; path: string } | { type: 'Unsupported'; typeName: string; toString: string }; +/** + * A value returned by an executed script, marshalled for the wire: everything + * `SerializedValue` covers, plus tables, which the plugin walks recursively + * (array-like tables stay arrays, anything else becomes an object with string + * keys). + * + * `{ type: 'Nil' }` stands in for a `nil` return value: a JSON array cannot + * hold a hole, and dropping the entry would silently shift every value after + * it, misreporting which value was returned where. + */ +export type SerializedReturnValue = + | SerializedValue + | { type: 'Nil' } + | SerializedReturnValue[] + | { [key: string]: SerializedReturnValue }; + export interface DataModelInstance { name: string; className: string; @@ -99,6 +115,13 @@ export interface ScriptCompleteMessage extends BaseMessage { success: boolean; error?: string; output?: Array<{ level: string; body: string; timestamp: number }>; + /** + * Everything the script returned, in order, marshalled by the plugin. + * Absent when the plugin never reported a return channel (an older plugin + * build, or a script that failed before returning) — distinct from `[]`, + * which means the script ran and returned nothing. + */ + returnValues?: SerializedReturnValue[]; }; } @@ -354,6 +377,11 @@ export function decodePluginMessage(raw: string): PluginMessage | null { timestamp: typeof e.timestamp === 'number' ? e.timestamp : 0, })) : undefined; + // The values themselves are whatever the script returned, so they are + // carried through as-is; only the array wrapper is checked. + const returnValues = Array.isArray(payload.returnValues) + ? (payload.returnValues as SerializedReturnValue[]) + : undefined; return { type: 'scriptComplete', sessionId, @@ -363,6 +391,7 @@ export function decodePluginMessage(raw: string): PluginMessage | null { error: typeof payload.error === 'string' ? payload.error : undefined, output, + returnValues, }, }; } diff --git a/tools/studio-bridge/templates/studio-bridge-plugin-test/test/execute-handler.test.luau b/tools/studio-bridge/templates/studio-bridge-plugin-test/test/execute-handler.test.luau index 30c3e5eb169..dc39d8ee465 100644 --- a/tools/studio-bridge/templates/studio-bridge-plugin-test/test/execute-handler.test.luau +++ b/tools/studio-bridge/templates/studio-bridge-plugin-test/test/execute-handler.test.luau @@ -338,6 +338,89 @@ table.insert(tests, { end, }) +table.insert(tests, { + name = "sequential queue: a request that throws while reporting is answered, not latched", + fn = function() + ExecuteAction._resetQueue() + local sentMessages = {} + local failNextSend = true + local sendMessage = function(msg) + if failNextSend then + failNextSend = false + error("encode failed") + end + table.insert(sentMessages, msg) + end + + local router = ActionRouter.new() + ExecuteAction.register(router, sendMessage) + + router:dispatch({ + type = "execute", + sessionId = "sess-wedge", + requestId = "req-w1", + payload = { code = "return 1" }, + }) + + assertEqual(#sentMessages, 1, "the failed request is reported rather than dropped") + assertEqual(sentMessages[1].requestId, "req-w1") + assertFalse(sentMessages[1].payload.success) + assertEqual(sentMessages[1].payload.code, "INTERNAL_ERROR") + + -- The queue must still drain: a latched _processing would leave this + -- request, and every later one, sitting in the queue forever. + router:dispatch({ + type = "execute", + sessionId = "sess-wedge", + requestId = "req-w2", + payload = { code = "print('innocent'); return 2" }, + }) + + assertEqual(#sentMessages, 2, "the next request still runs") + assertEqual(sentMessages[2].requestId, "req-w2") + assertTrue(sentMessages[2].payload.success) + assertEqual(sentMessages[2].payload.returnValues[1], 2) + end, +}) + +table.insert(tests, { + name = "sequential queue: drains even when the failure itself cannot be reported", + fn = function() + ExecuteAction._resetQueue() + + local deadRouter = ActionRouter.new() + ExecuteAction.register(deadRouter, function() + error("encode failed") + end) + + deadRouter:dispatch({ + type = "execute", + sessionId = "sess-wedge2", + requestId = "req-w3", + payload = { code = "return 1" }, + }) + + -- Nothing could be delivered for req-w3, so the only observable question + -- is whether the queue was released for whoever comes next. + local sentMessages = {} + local liveRouter = ActionRouter.new() + ExecuteAction.register(liveRouter, function(msg) + table.insert(sentMessages, msg) + end) + + liveRouter:dispatch({ + type = "execute", + sessionId = "sess-wedge2", + requestId = "req-w4", + payload = { code = "return 4" }, + }) + + assertEqual(#sentMessages, 1, "a later request still gets a response") + assertEqual(sentMessages[1].requestId, "req-w4") + assertTrue(sentMessages[1].payload.success) + end, +}) + -- =========================================================================== -- sendMessage callback integration -- =========================================================================== @@ -403,4 +486,180 @@ table.insert(tests, { end, }) +-- =========================================================================== +-- Return value marshalling +-- =========================================================================== + +table.insert(tests, { + name = "returnValues: empty when the script returns nothing", + fn = function() + local result = ExecuteAction.handleExecute({ code = "local x = 1" }, "req-rv0", "sess-rv0") + assertNotNil(result.returnValues, "returnValues should be present on a successful run") + assertEqual(#result.returnValues, 0, "no values returned") + end, +}) + +table.insert(tests, { + name = "returnValues: carries every returned value in order", + fn = function() + local result = ExecuteAction.handleExecute({ code = "return 'str', 42, true" }, "req-rv1", "sess-rv1") + assertEqual(#result.returnValues, 3, "three values returned") + assertEqual(result.returnValues[1], "str") + assertEqual(result.returnValues[2], 42) + assertEqual(result.returnValues[3], true) + end, +}) + +table.insert(tests, { + name = "returnValues: nested tables survive as tables", + fn = function() + local result = ExecuteAction.handleExecute( + { code = "return { slug = 'maid', counts = { passed = 1014, failed = 0 } }" }, + "req-rv2", + "sess-rv2" + ) + assertEqual(#result.returnValues, 1, "one value returned") + local value = result.returnValues[1] + assertEqual(value.slug, "maid") + assertEqual(value.counts.passed, 1014) + assertEqual(value.counts.failed, 0) + end, +}) + +table.insert(tests, { + name = "returnValues: array-like tables stay arrays", + fn = function() + local result = ExecuteAction.handleExecute({ code = "return { 10, 20, 30 }" }, "req-rv3", "sess-rv3") + local value = result.returnValues[1] + assertEqual(#value, 3, "array length") + assertEqual(value[1], 10) + assertEqual(value[3], 30) + end, +}) + +table.insert(tests, { + name = "returnValues: non-string keys become strings so the table can be encoded", + fn = function() + local result = ExecuteAction.handleExecute({ code = "return { [1] = 'a', name = 'b' }" }, "req-rv4", "sess-rv4") + local value = result.returnValues[1] + assertEqual(value["1"], "a", "numeric key stringified") + assertEqual(value.name, "b") + end, +}) + +table.insert(tests, { + name = "returnValues: a returned nil keeps its position", + fn = function() + local result = ExecuteAction.handleExecute({ code = "return nil, 7" }, "req-rv5", "sess-rv5") + assertEqual(#result.returnValues, 2, "two values returned") + assertEqual(result.returnValues[1].type, "Nil", "nil marker") + assertEqual(result.returnValues[2], 7) + end, +}) + +table.insert(tests, { + name = "returnValues: a cycle is reported instead of recursing forever", + fn = function() + local result = + ExecuteAction.handleExecute({ code = "local t = {}; t.self = t; return t" }, "req-rv6", "sess-rv6") + local value = result.returnValues[1] + assertEqual(value.self.type, "Unsupported", "cycle is marked unsupported") + assertContains(value.self.toString, "cycle") + end, +}) + +table.insert(tests, { + name = "returnValues: non-finite numbers are marked, not emitted as invalid JSON", + fn = function() + local result = ExecuteAction.handleExecute({ code = "return math.huge, 0 / 0" }, "req-rv7", "sess-rv7") + assertEqual(result.returnValues[1].type, "Unsupported", "inf is marked") + assertEqual(result.returnValues[1].typeName, "number") + assertEqual(result.returnValues[2].type, "Unsupported", "nan is marked") + end, +}) + +table.insert(tests, { + name = "returnValues: functions are marked unsupported rather than dropped", + fn = function() + local result = ExecuteAction.handleExecute({ code = "return function() end" }, "req-rv8", "sess-rv8") + assertEqual(#result.returnValues, 1, "one value returned") + assertEqual(result.returnValues[1].type, "Unsupported") + assertEqual(result.returnValues[1].typeName, "function") + end, +}) + +table.insert(tests, { + name = "returnValues: a hostile key metamethod degrades to a marker instead of throwing", + fn = function() + local result = ExecuteAction.handleExecute({ + code = "local k = setmetatable({}, { __tostring = function() error('bad tostring') end }); return { [k] = 1, name = 2 }", + }, "req-rv10", "sess-rv10") + assertTrue(result.success, "the run itself still succeeded") + assertEqual(#result.returnValues, 1, "one marker in place of the values") + assertEqual(result.returnValues[1].type, "Unsupported") + assertContains(result.returnValues[1].toString, "not serializable") + end, +}) + +table.insert(tests, { + name = "returnValues: __iter is bypassed, so real contents are marshalled", + fn = function() + local result = ExecuteAction.handleExecute({ + code = "return setmetatable({ real = 'x' }, { __iter = function() error('bad iter') end })", + }, "req-rv11", "sess-rv11") + assertTrue(result.success, "should succeed") + assertEqual(result.returnValues[1].real, "x", "raw contents, not whatever __iter yields") + end, +}) + +table.insert(tests, { + name = "returnValues: shared references cannot explode past the node budget", + fn = function() + -- Only 30 levels deep, but 2^30 nodes if every shared edge is expanded. + local result = ExecuteAction.handleExecute( + { code = "local x = {}; for _ = 1, 30 do x = { a = x, b = x } end; return x" }, + "req-rv12", + "sess-rv12" + ) + assertTrue(result.success, "should succeed") + + local function findBudgetMarker(value: any): boolean + if type(value) ~= "table" then + return false + end + if value.type == "Unsupported" and string.find(tostring(value.toString), "budget", 1, true) then + return true + end + for _, item in value do + if findBudgetMarker(item) then + return true + end + end + return false + end + + assertTrue(findBudgetMarker(result.returnValues[1]), "walk stopped at the budget") + end, +}) + +table.insert(tests, { + name = "returnValues: keys that collapse onto one JSON key are marked, not silently dropped", + fn = function() + local result = + ExecuteAction.handleExecute({ code = "return { [1] = 'a', ['1'] = 'b' }" }, "req-rv13", "sess-rv13") + local value = result.returnValues[1] + assertEqual(value["1"].type, "Unsupported", "collision is marked") + assertEqual(value["1"].typeName, "collision") + end, +}) + +table.insert(tests, { + name = "returnValues: absent on a failed run", + fn = function() + local result = ExecuteAction.handleExecute({ code = "error('boom')" }, "req-rv9", "sess-rv9") + assertFalse(result.success) + assertNil(result.returnValues, "a failed run reports no return values") + end, +}) + return tests From a79ce464b7e06a2398eb9f2fd323fefe7d550eda Mon Sep 17 00:00:00 2001 From: James Onnen Date: Tue, 11 Aug 2026 17:41:11 -0700 Subject: [PATCH 2/2] feat(nevermore-cli): surface a run's structured result on ScriptRunResult MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test runs learn everything about themselves by scraping the engine's log output, and Open Cloud truncates those logs on a long run — a script printing 20,001 lines came back with 7,627 of them, and the window size varies, so there is no threshold to stay under. Results have to travel as a value instead of as text. ScriptRunResult gains returnValues, filled in by both transports: the cloud context reads the task's output.results, the local context takes what the Studio bridge marshalled. LuauTask.output was typed as an array of { value?: string } wrappers, which does not exist — the results are a flat array of natively typed values, so a returned Lua table is real nested JSON. Nothing read it, so nothing noticed. returnValues is absent, not empty, when a transport delivered no result at all: an oversize return value fails a cloud task outright rather than truncating it (~4MB fails, ~2MB arrives intact), leaving no output and no error message, and a later consumer needs to tell that apart from a script that returned nothing so it knows when falling back to the logs is worth it. Nothing consumes the new field yet. --- docs/gotchas/tooling.md | 4 + .../job-context/batch-script-job-context.ts | 3 + .../job-context/cloud-job-context.test.ts | 112 ++++++++++++++++++ .../utils/job-context/cloud-job-context.ts | 2 + .../src/utils/job-context/job-context.ts | 21 ++++ .../job-context/local-job-context.test.ts | 89 ++++++++++++++ .../utils/job-context/local-job-context.ts | 2 +- .../open-cloud/open-cloud-client.test.ts | 50 +++++++- .../src/utils/open-cloud/open-cloud-client.ts | 30 ++++- 9 files changed, 309 insertions(+), 4 deletions(-) create mode 100644 tools/nevermore-cli/src/utils/job-context/cloud-job-context.test.ts create mode 100644 tools/nevermore-cli/src/utils/job-context/local-job-context.test.ts diff --git a/docs/gotchas/tooling.md b/docs/gotchas/tooling.md index 4bcd9ecc25c..6027299b44a 100644 --- a/docs/gotchas/tooling.md +++ b/docs/gotchas/tooling.md @@ -33,6 +33,10 @@ When a section grows to 10+ items, graduate it to its own doc. - **The watch service and the lock file do not share a version vocabulary.** A watch's `baselineVersion`, and the `currentVersion` it reports back, are the *asset-delivery content hash* for the place — that's what the service's Roblox driver reads, and it compares them by string equality. `deploy.nevermore.lock.json` holds the *Open Cloud place version* (an integer). They are never equal, so handing the lock's number over as a baseline reads as drift on the very first poll and dispatches a rebuild of the build that just shipped. The CLI therefore sends no baseline at all — the service's first poll adopts what it sees, which is what a baseline was for — and treats every version the service reports as an opaque "something moved" token, asking Open Cloud what the place is actually at before deciding to rebuild. Anything comparing a service version against a lock version is wrong even when the types line up. +- **Open Cloud truncates long engine logs, so anything that must be read back exactly belongs in the script's return value.** A script printing 20,001 lines came back with 7,627 of them — the last contiguous block, with the head dropped — and the size of the window varies run to run (6,715 and 3,236 lines on other runs), so there is no line count to stay under. A Luau execution task's return value is not subject to this: it arrives on the task as `output.results`, a flat array of the returned values (`return t, "s", 42` → three entries), and Roblox serializes them itself, so a returned Lua table is real nested JSON. Do **not** `HttpService:JSONEncode` the result — that lands as a double-encoded string. + +- **An oversize return value annihilates the task rather than truncating the value.** A ~2.1MB return value arrives complete and intact; a ~4.2MB one fails the whole task — state `FAILED`, no `output`, and no error message saying why. So code reading a return value has to treat "the task reported no output" as *unknown* rather than empty, and fall back to the logs; `getTaskReturnValues` in `open-cloud-client.ts` draws exactly that line (`undefined` = nothing came back, `[]` = the script returned nothing). + - **`--script-text` loses everything after the first line when invoked through `npx` on Windows**: the `npx.cmd` shim truncates a multi-line argument, so `nevermore test --cloud --script-text '\n'` silently runs only line 1 (and prints `(no output)` when line 1 produced none). Either write the script as a single line with `;` separators, or bypass the shim: `node tools/nevermore-cli/dist/nevermore.js test --cloud --script-text '...'`, which passes newlines through intact. ## Claude Code hooks diff --git a/tools/nevermore-cli/src/utils/job-context/batch-script-job-context.ts b/tools/nevermore-cli/src/utils/job-context/batch-script-job-context.ts index c4bc5a56303..bf0fe6e2c4b 100644 --- a/tools/nevermore-cli/src/utils/job-context/batch-script-job-context.ts +++ b/tools/nevermore-cli/src/utils/job-context/batch-script-job-context.ts @@ -135,6 +135,9 @@ export class BatchScriptJobContext implements JobContext { success: result.success, durationMs: result.durationMs, errorMessage: result.error, + // returnValues stays absent: one execution covers every package, so the + // single return value it produces has to be split per package before any + // of it can surface here. }; } diff --git a/tools/nevermore-cli/src/utils/job-context/cloud-job-context.test.ts b/tools/nevermore-cli/src/utils/job-context/cloud-job-context.test.ts new file mode 100644 index 00000000000..70a3e1d6635 --- /dev/null +++ b/tools/nevermore-cli/src/utils/job-context/cloud-job-context.test.ts @@ -0,0 +1,112 @@ +/** + * Unit tests for CloudJobContext.runScriptAsync — validates what a finished + * Open Cloud task reports back as a ScriptRunResult, in particular that a task + * which produced no output stays distinguishable from one that returned nothing. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { type Reporter } from '@quenty/cli-output-helpers/reporting'; +import { CloudJobContext } from './cloud-job-context.js'; +import { + type LuauTask, + type OpenCloudClient, +} from '../open-cloud/open-cloud-client.js'; +import { type Deployment } from './job-context.js'; + +function createReporter(): Reporter { + return { + onPackagePhaseChange: vi.fn(), + onPackageProgressUpdate: vi.fn(), + onPackageStart: vi.fn(), + onPackageResult: vi.fn(), + } as unknown as Reporter; +} + +/** + * A client whose task completes as `completedTask`. The real deployment handle + * is private to the context, so the test passes the three fields runScriptAsync + * reads off it. + */ +function createContext(completedTask: Partial) { + const task = { + path: 'universes/1/places/2/versions/3/luau-execution-session-tasks/4', + createTime: '2026-01-01T00:00:00Z', + updateTime: '2026-01-01T00:01:00Z', + user: 'users/1', + state: 'COMPLETE', + script: 'return 1', + ...completedTask, + } as LuauTask; + + const client = { + createExecutionTaskAsync: vi.fn(async () => task), + pollTaskCompletionAsync: vi.fn(async () => task), + } as unknown as OpenCloudClient; + + const context = new CloudJobContext(createReporter(), client); + const deployment = { + universeId: 1, + placeId: 2, + version: 3, + } as unknown as Deployment; + + return { context, deployment }; +} + +describe('CloudJobContext.runScriptAsync', () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it('reports the values the script returned', async () => { + // Fake timers keep the client-side timeout race from leaving a live timer + // behind; nothing in the run itself waits on one. + vi.useFakeTimers(); + const { context, deployment } = createContext({ + state: 'COMPLETE', + output: { results: [{ slug: 'maid', counts: { passed: 1014 } }] }, + }); + + const result = await context.runScriptAsync(deployment, { + scriptContent: 'return results', + packageName: 'maid', + }); + + expect(result.success).toBe(true); + expect(result.returnValues).toEqual([ + { slug: 'maid', counts: { passed: 1014 } }, + ]); + }); + + it('reports an empty result when the task returned nothing', async () => { + vi.useFakeTimers(); + const { context, deployment } = createContext({ + state: 'COMPLETE', + output: {}, + }); + + const result = await context.runScriptAsync(deployment, { + scriptContent: 'print("hi")', + packageName: 'maid', + }); + + expect(result.returnValues).toEqual([]); + }); + + it('leaves returnValues absent when a failed task carried no output', async () => { + vi.useFakeTimers(); + const { context, deployment } = createContext({ + state: 'FAILED', + output: undefined, + }); + + const result = await context.runScriptAsync(deployment, { + scriptContent: 'return huge', + packageName: 'maid', + }); + + expect(result.success).toBe(false); + expect(result.taskState).toBe('FAILED'); + expect(result.returnValues).toBeUndefined(); + }); +}); diff --git a/tools/nevermore-cli/src/utils/job-context/cloud-job-context.ts b/tools/nevermore-cli/src/utils/job-context/cloud-job-context.ts index fe5455e60c2..2375bee5e5a 100644 --- a/tools/nevermore-cli/src/utils/job-context/cloud-job-context.ts +++ b/tools/nevermore-cli/src/utils/job-context/cloud-job-context.ts @@ -1,5 +1,6 @@ import { type Reporter } from '@quenty/cli-output-helpers/reporting'; import { + getTaskReturnValues, type LuauTask, type OpenCloudClient, } from '../open-cloud/open-cloud-client.js'; @@ -134,6 +135,7 @@ export class CloudJobContext extends BaseJobContext { success: completedTask.state === 'COMPLETE', taskState: completedTask.state, errorMessage, + returnValues: getTaskReturnValues(completedTask), }; } diff --git a/tools/nevermore-cli/src/utils/job-context/job-context.ts b/tools/nevermore-cli/src/utils/job-context/job-context.ts index c5b95969442..7d9009807a8 100644 --- a/tools/nevermore-cli/src/utils/job-context/job-context.ts +++ b/tools/nevermore-cli/src/utils/job-context/job-context.ts @@ -28,6 +28,27 @@ export interface ScriptRunResult { taskState?: string; /** Error message from the execution backend, if any. */ errorMessage?: string; + /** + * Everything the executed script returned, in order — the structured channel + * out of a run, as opposed to its printed output. Engine logs are truncated + * by Open Cloud on long runs, so anything a caller must read back exactly + * belongs here rather than in the log text. + * + * `undefined` means the transport never delivered a return channel: a cloud + * task that ended without an `output` (a FAILED task carries none, and an + * oversize return value fails the task rather than truncating the value), a + * bridge run that timed out or disconnected, or a context that does not + * carry return values at all. That is deliberately distinct from `[]`, which + * means the script ran and returned nothing — a caller that needs the value + * can fall back to parsing logs in the first case but not the second. + * + * Values are JSON-shaped, but the two transports spell exotic Luau types + * differently: Open Cloud auto-serializes them, while the Studio bridge + * marshals them into `{ type, value }` wrappers (`SerializedReturnValue`). + * Plain tables of strings, numbers and booleans come back identically on + * both, so structured results should stay inside that subset. + */ + returnValues?: unknown[]; } /** diff --git a/tools/nevermore-cli/src/utils/job-context/local-job-context.test.ts b/tools/nevermore-cli/src/utils/job-context/local-job-context.test.ts new file mode 100644 index 00000000000..2f5883f0ed5 --- /dev/null +++ b/tools/nevermore-cli/src/utils/job-context/local-job-context.test.ts @@ -0,0 +1,89 @@ +/** + * Unit tests for LocalJobContext.runScriptAsync — validates that a bridge run + * hands its script return values on as a ScriptRunResult, and that a run which + * never completed reports none rather than an empty result. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { type Reporter } from '@quenty/cli-output-helpers/reporting'; +import { LocalJobContext } from './local-job-context.js'; +import { type Deployment } from './job-context.js'; + +function createReporter(): Reporter { + return { + onPackagePhaseChange: vi.fn(), + onPackageProgressUpdate: vi.fn(), + onPackageStart: vi.fn(), + onPackageResult: vi.fn(), + } as unknown as Reporter; +} + +/** + * The deployment handle is private to the context, so the test stands in a + * bridge with the one method runScriptAsync calls on it. + */ +function createDeployment( + executeAsync: () => Promise<{ + success: boolean; + logs: string; + returnValues?: unknown[]; + }> +): Deployment { + return { + bridge: { executeAsync }, + cachedLogs: '', + } as unknown as Deployment; +} + +describe('LocalJobContext.runScriptAsync', () => { + it('reports the values the script returned', async () => { + const context = new LocalJobContext(createReporter()); + const deployment = createDeployment(async () => ({ + success: true, + logs: 'ran', + returnValues: [{ counts: { passed: 7 } }], + })); + + const result = await context.runScriptAsync(deployment, { + scriptContent: 'return results', + packageName: 'maid', + }); + + expect(result.success).toBe(true); + expect(result.returnValues).toEqual([{ counts: { passed: 7 } }]); + expect(await context.getLogsAsync(deployment)).toBe('ran'); + }); + + it('leaves returnValues absent when the bridge reported none', async () => { + const context = new LocalJobContext(createReporter()); + const deployment = createDeployment(async () => ({ + success: false, + logs: '[StudioBridge] Timed out after 200ms', + })); + + const result = await context.runScriptAsync(deployment, { + scriptContent: 'while true do end', + packageName: 'maid', + }); + + expect(result.returnValues).toBeUndefined(); + }); + + it('leaves returnValues absent when the bridge throws', async () => { + const context = new LocalJobContext(createReporter()); + const deployment = createDeployment(async () => { + throw new Error('no connected client'); + }); + + const result = await context.runScriptAsync(deployment, { + scriptContent: 'return results', + packageName: 'maid', + }); + + expect(result.success).toBe(false); + expect(result.returnValues).toBeUndefined(); + expect(await context.getLogsAsync(deployment)).toContain( + 'no connected client' + ); + }); +}); diff --git a/tools/nevermore-cli/src/utils/job-context/local-job-context.ts b/tools/nevermore-cli/src/utils/job-context/local-job-context.ts index 3c113823826..bbc268dad9b 100644 --- a/tools/nevermore-cli/src/utils/job-context/local-job-context.ts +++ b/tools/nevermore-cli/src/utils/job-context/local-job-context.ts @@ -69,7 +69,7 @@ export class LocalJobContext extends BaseJobContext { timeoutMs, }); localDeployment.cachedLogs = result.logs; - return { success: result.success }; + return { success: result.success, returnValues: result.returnValues }; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); diff --git a/tools/nevermore-cli/src/utils/open-cloud/open-cloud-client.test.ts b/tools/nevermore-cli/src/utils/open-cloud/open-cloud-client.test.ts index e5148aa1a7c..ed1c5687aa3 100644 --- a/tools/nevermore-cli/src/utils/open-cloud/open-cloud-client.test.ts +++ b/tools/nevermore-cli/src/utils/open-cloud/open-cloud-client.test.ts @@ -1,6 +1,10 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as fs from 'fs/promises'; -import { OpenCloudClient } from './open-cloud-client.js'; +import { + OpenCloudClient, + getTaskReturnValues, + type LuauTask, +} from './open-cloud-client.js'; import type { RateLimiter } from './rate-limiter.js'; vi.mock('@quenty/cli-output-helpers', () => ({ @@ -367,3 +371,47 @@ describe('OpenCloudClient.resolveLatestPlaceVersionAsync', () => { ).rejects.toThrowError(/unparseable version path/); }); }); + +describe('getTaskReturnValues', () => { + function makeTask(overrides: Partial): LuauTask { + return { + path: 'universes/1/places/2/versions/3/luau-execution-session-tasks/4', + createTime: '2026-01-01T00:00:00Z', + updateTime: '2026-01-01T00:01:00Z', + user: 'users/1', + state: 'COMPLETE', + script: 'return 1', + ...overrides, + }; + } + + it('returns the values natively typed, one entry per returned value', () => { + // Roblox serializes the return value itself, so a returned table arrives as + // real nested JSON — there is no JSON string to parse a second time. + const task = makeTask({ + output: { + results: [{ slug: 'maid', counts: { passed: 1014 } }, 'str', 42, true], + }, + }); + + expect(getTaskReturnValues(task)).toEqual([ + { slug: 'maid', counts: { passed: 1014 } }, + 'str', + 42, + true, + ]); + }); + + it('reports an empty result when the task returned nothing', () => { + expect(getTaskReturnValues(makeTask({ output: {} }))).toEqual([]); + }); + + it('reports undefined when a failed task carried no output at all', () => { + // An oversize return value fails the task with no output and no error + // message, so "nothing came back to read" has to stay distinguishable from + // "the script returned nothing" — only the former can fall back to logs. + const task = makeTask({ state: 'FAILED', output: undefined }); + + expect(getTaskReturnValues(task)).toBeUndefined(); + }); +}); diff --git a/tools/nevermore-cli/src/utils/open-cloud/open-cloud-client.ts b/tools/nevermore-cli/src/utils/open-cloud/open-cloud-client.ts index 3deaf3169e1..f04b3ce7299 100644 --- a/tools/nevermore-cli/src/utils/open-cloud/open-cloud-client.ts +++ b/tools/nevermore-cli/src/utils/open-cloud/open-cloud-client.ts @@ -22,12 +22,38 @@ export interface LuauTask { | 'FAILED'; script: string; timeout?: string; - /** Script return value (populated on COMPLETE). */ - output?: { results?: Array<{ value?: string }> }; + /** + * What the script returned, populated on COMPLETE. `results` is a flat array + * of the returned values — `return t, "s", 42` arrives as three entries — + * natively typed: Roblox serializes them itself, so a returned Lua table is + * real nested JSON here, not a JSON string. (Which is also why a script must + * not JSONEncode its result: that lands as a double-encoded string.) + * + * Absent whenever the task produced no result. A FAILED task carries no + * `output` at all, with no error message explaining why — and an oversize + * return value (~4MB observed; ~2MB still arrives complete) fails the task + * exactly that way rather than truncating the value. + */ + output?: { results?: unknown[] }; /** Error details (populated on FAILED). */ error?: { code?: string; message?: string }; } +/** + * The values a finished task's script returned, or `undefined` when the task + * reported no result at all. + * + * The distinction matters: `undefined` means nothing came back to read, so a + * caller can fall back to the task's logs, while `[]` means the task did report + * a result and the script returned nothing — no fallback will find more. + */ +export function getTaskReturnValues(task: LuauTask): unknown[] | undefined { + if (!task.output) { + return undefined; + } + return task.output.results ?? []; +} + export interface OpenCloudClientOptions { apiKey: string | (() => Promise); rateLimiter: RateLimiter;