Conversation
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4b844437f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function stringifyJsonObject(payload: Record<string, unknown>, field: string): string { | ||
| try { | ||
| return JSON.stringify(payload); | ||
| } catch { |
There was a problem hiding this comment.
Reject undefined JSON serialization results
stringifyJsonObject() assumes JSON.stringify(payload) always returns a string, but for objects with a custom toJSON() that returns undefined, it returns undefined instead of throwing. In this path, apps.triggerEvent will later call Buffer.byteLength on undefined and throw a raw TypeError, and apps.push will incorrectly accept a payload that is not valid JSON object output. Please treat non-string stringify results as INVALID_ARGS so payload validation remains deterministic.
Useful? React with 👍 / 👎.
a4b8444 to
2ad0d17
Compare
Summary
Add Phase 1 runtime app lifecycle commands after #412:
device.apps.open,close,list,state,push, andtriggerEventcommands,bindCommands(), andcreateCommandRouter()toJSON()edge cases, and payload size limitsCOMMAND_OWNERSHIP.mdTouched files: 14. Scope stayed within the runtime command/API, conformance, and docs follow-up to #412; daemon handler migration is intentionally deferred.
Follow-up work
Next PR should wire existing daemon/platform semantics onto these runtime APIs:
openApp,closeApp,listApps,getAppState,pushFile, andtriggerAppEventopen,close,apps,appstate,push, andtrigger-app-eventcompatibility paths to call the runtime where session/device resolution already existsapps.stateshould support a session foreground-app fallback or remain explicitapponlyapps.pushartifact inputs and restricted local path behaviorBackendOpenTargetshould become a stricter discriminated union once cloud adapter usage is knownValidation
pnpm formatpnpm check:quickpnpm check:unit