Skip to content

Commit 7531470

Browse files
committed
refactor(desktop): define the codecs of every payload this change touched
`desktop/AGENTS.md` requires an explicit `FromJson`/`ToJson` for anything crossing the JSON boundary, and requires replacing a derived codec in the same change that modifies its type. This change gave `agent.start`, `agent.compact`, `agent.goal`, `session.load`(`_archived`) and `session.archive`/`unarchive` a required `workspace`, and all five still derived theirs. The five codecs now say what the derive left implicit, and say it the same way the neighbouring payloads in this file already do. A required field must be present and of its type; `null` is not a value for one, so it is refused rather than read as absence — which for `workspace` is the point, since reading it as an absent field would leave the host guessing which store was meant. An optional field reads `null` and absence alike as `None` and refuses any other type, so a `null` `text` still clears a goal rather than setting an empty one. Unknown fields are ignored, which is what lets a host accept and drop a client-authored `session_root`. The encoder writes every required field and omits an absent optional instead of emitting `null`. Reported by Codex on #886.
1 parent 0f4ff34 commit 7531470

3 files changed

Lines changed: 385 additions & 14 deletions

File tree

desktop/internal/api/payload.mbt

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,85 @@ test "a run payload without a store is not a valid request" {
195195
assert_true(goal is None)
196196
}
197197

198+
///|
199+
test "a required field is not satisfied by null or a wrong type" {
200+
// `null` is not a store: reading it as an absent field would leave the host
201+
// guessing which store the request meant, the silent misplacement the
202+
// required field exists to prevent.
203+
let null_store : @protocol.SessionPayload? = Some(
204+
@json.from_json({ "session": "s-move", "workspace": Json::null() }),
205+
) catch {
206+
_ => None
207+
}
208+
assert_true(null_store is None)
209+
let wrong_type : @protocol.LoadSessionPayload? = Some(
210+
@json.from_json({ "session": "s-load", "workspace": 7 }),
211+
) catch {
212+
_ => None
213+
}
214+
assert_true(wrong_type is None)
215+
}
216+
217+
///|
218+
test "an absent optional reads the same as an explicit null" {
219+
let absent : @protocol.StartPayload = @json.from_json({
220+
"task": "hello",
221+
"session": "s-start",
222+
"workspace": "",
223+
})
224+
let explicit : @protocol.StartPayload = @json.from_json({
225+
"task": "hello",
226+
"session": "s-start",
227+
"workspace": "",
228+
"model": Json::null(),
229+
"max_steps": Json::null(),
230+
"submission_id": Json::null(),
231+
})
232+
assert_true(
233+
absent.model is None &&
234+
absent.max_steps is None &&
235+
absent.submission_id is None,
236+
)
237+
assert_true(
238+
explicit.model is None &&
239+
explicit.max_steps is None &&
240+
explicit.submission_id is None,
241+
)
242+
// A malformed optional is still refused: absence is a value, a number is a
243+
// client defect.
244+
let malformed : @protocol.StartPayload? = Some(
245+
@json.from_json({
246+
"task": "hello",
247+
"session": "s-start",
248+
"workspace": "",
249+
"model": 7,
250+
}),
251+
) catch {
252+
_ => None
253+
}
254+
assert_true(malformed is None)
255+
}
256+
257+
///|
258+
test "an absent optional is encoded as an absent key, never as null" {
259+
let payload : @protocol.GoalPayload = {
260+
session: "s-goal",
261+
workspace: "",
262+
text: None,
263+
auto: None,
264+
model: None,
265+
max_steps: None,
266+
}
267+
guard payload.to_json() is Object(fields) else {
268+
fail("goal payload is a JSON object")
269+
}
270+
assert_eq(fields.get("session"), Some(Json::string("s-goal")))
271+
assert_eq(fields.get("workspace"), Some(Json::string("")))
272+
for name in ["text", "auto", "model", "max_steps"] {
273+
assert_true(fields.get(name) is None)
274+
}
275+
}
276+
198277
///|
199278
test "goal payload distinguishes set from clear by the text field" {
200279
let set : @protocol.GoalPayload = @json.from_json({

0 commit comments

Comments
 (0)