Skip to content

fix(execution-history): tolerate runs stored before the actor fields - #23

Merged
aryasaatvik merged 2 commits into
devfrom
fix-runs-legacy-actor-keys
Jun 14, 2026
Merged

fix(execution-history): tolerate runs stored before the actor fields#23
aryasaatvik merged 2 commits into
devfrom
fix-runs-legacy-actor-keys

Conversation

@aryasaatvik

Copy link
Copy Markdown
Owner

The bug

The runs list returns an empty-body 400 for everyone. Root cause is a schema/data-evolution bug from #20, not anything resource-related:

RunRow.actorId/actorLabel/actorKind were Schema.NullOr(...) — present-but-nullable, which means the key must exist. Runs are stored as JSON documents, and every run written before #20 has a data doc with no such key. The store passes stored docs straight to the HTTP response encoder, which rejects the newest row with Missing key at ["runs"][0]["actorId"]. A response-encode failure emits a bare 400 (no body, no content-type) — the empty-body signature. Since no executions have happened since the actor store deployed, 100% of stored runs are legacy and every request fails.

The fix

Make the three actor keys optional with a decoding default of null:

actorId: Schema.optional(Schema.NullOr(Schema.String)).pipe(
  Schema.withDecodingDefaultType(Effect.succeed(null)),
)
  • Decoded type stays string | null (always present) — every reader still treats the field as required; it just defaults to null for pre-actor runs.
  • Wire/storage tolerates an absent key — legacy docs decode and encode unchanged.
  • Immunizes the collection against the next field added the same way.

No data migration, no prod writes — deploy-forward and the page is back with all run history intact.

Verification

  • New regression test round-trips a legacy-shaped doc (decode → null; encode with the keys absent succeeds — the exact failure).
  • execution-history suite 20/20, typecheck 41/41, lint + format clean.

(Build/Deploy-preview checks fail on the fork's missing R2 secrets, as on prior PRs — unrelated.)

RunRow's actorId/actorLabel/actorKind were NullOr (key required), but runs are
JSON documents and rows written before #20 have no such key at all — so the HTTP
response encoder rejected the newest page ("Missing key at runs[0].actorId") and
the entire runs list 400'd with an empty body.

Make the keys optional with a decoding default of null: the decoded type stays
`string | null` (always present, so every reader still treats it as required)
while legacy docs decode AND encode unchanged. This also immunizes the collection
against the next field added the same way. Covered by a legacy-doc encode/decode
regression test.
@greptile-apps

greptile-apps Bot commented Jun 14, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a schema-evolution bug where the three actor fields (actorId, actorLabel, actorKind) introduced in #20 used Schema.NullOr — requiring the key to exist — which caused the response encoder to emit a bare 400 for every pre-#20 run document that lacks those keys entirely.

  • collections.ts: Actor fields changed to Schema.optional(NullOr).pipe(withDecodingDefaultType(Effect.succeed(null))), so missing keys decode to null while the TypeScript decoded type stays string | null (never undefined) — no reader changes needed.
  • collections.test.ts: New test file exercises all three paths: decode of a legacy doc (keys absent → null), direct encode of a legacy doc (the exact failure regression), and a full decode→encode round-trip for a run with actor fields set.

Confidence Score: 5/5

Safe to merge — the change is minimal, targeted, and directly addresses the 400 regression with no data migration required.

The fix makes the three actor fields optional at the wire level while keeping the decoded TypeScript type as string | null (always present), so no readers need updating. The regression test directly encodes a legacy-shaped document — the exact path that was 400ing — confirming the failure mode is resolved. The round-trip test for attributed runs confirms existing populated values are unaffected.

No files require special attention.

Important Files Changed

Filename Overview
packages/plugins/execution-history/src/sdk/collections.ts Actor fields changed from Schema.NullOr (required key, nullable value) to Schema.optional(NullOr).pipe(withDecodingDefaultType) — absent keys now decode to null, fixing the 400 for legacy run documents.
packages/plugins/execution-history/src/sdk/collections.test.ts New regression test suite covering decode of legacy doc (keys absent to null), encode of legacy doc (the exact failure path), and full round-trip of a run with actor fields populated.

Sequence Diagram

sequenceDiagram
    participant Store as KV Store
    participant Encoder as Response Encoder (RunRow)
    participant Client

    note over Store: Legacy doc (no actorId/actorLabel/actorKind keys)
    Store->>Encoder: raw JSON doc
    alt Before fix (Schema.NullOr - key required)
        Encoder-->>Client: 400 Missing key at runs[0].actorId
    else After fix (Schema.optional + decoding default null)
        Encoder->>Encoder: absent key treated as optional, no error
        Encoder-->>Client: 200 with actorId absent
    end

    note over Store: New doc with actorId set
    Store->>Encoder: raw JSON doc with actor keys
    Encoder-->>Client: 200 with actorId present
Loading

Reviews (2): Last reviewed commit: "test(execution-history): assert actor-ke..." | Re-trigger Greptile

Comment thread packages/plugins/execution-history/src/sdk/collections.test.ts
Comment thread packages/plugins/execution-history/src/sdk/collections.test.ts Outdated
…e comment

Strengthen the legacy-doc regression test to assert the encoder leaves absent
actor keys absent (not fabricated) and carries present keys through encode
unchanged — so a regression in how the optional actor keys encode is caught, not
just a bare executionId check. Also drops a stale `optionalWith` comment.
@aryasaatvik
aryasaatvik merged commit 75a2273 into dev Jun 14, 2026
10 of 13 checks passed
@aryasaatvik
aryasaatvik deleted the fix-runs-legacy-actor-keys branch June 14, 2026 21:45
aryasaatvik added a commit that referenced this pull request Jun 23, 2026
…23)

## The bug

The runs list returns an **empty-body 400** for everyone. Root cause is
a schema/data-evolution bug from #20, not anything resource-related:

`RunRow.actorId/actorLabel/actorKind` were `Schema.NullOr(...)` —
present-but-nullable, which means **the key must exist**. Runs are
stored as JSON documents, and every run written *before* #20 has a
`data` doc with no such key. The store passes stored docs straight to
the HTTP response encoder, which rejects the newest row with `Missing
key at ["runs"][0]["actorId"]`. A response-encode failure emits a bare
400 (no body, no content-type) — the empty-body signature. Since no
executions have happened since the actor store deployed, 100% of stored
runs are legacy and every request fails.

## The fix

Make the three actor keys optional with a **decoding default of null**:

```ts
actorId: Schema.optional(Schema.NullOr(Schema.String)).pipe(
  Schema.withDecodingDefaultType(Effect.succeed(null)),
)
```

- **Decoded type stays `string | null`** (always present) — every reader
still treats the field as required; it just defaults to `null` for
pre-actor runs.
- **Wire/storage tolerates an absent key** — legacy docs decode *and*
encode unchanged.
- Immunizes the collection against the next field added the same way.

No data migration, no prod writes — deploy-forward and the page is back
with all run history intact.

## Verification

- New regression test round-trips a legacy-shaped doc (decode → null;
**encode with the keys absent succeeds** — the exact failure).
- execution-history suite 20/20, typecheck 41/41, lint + format clean.

(Build/Deploy-preview checks fail on the fork's missing R2 secrets, as
on prior PRs — unrelated.)
aryasaatvik added a commit that referenced this pull request Jun 23, 2026
…23)

## The bug

The runs list returns an **empty-body 400** for everyone. Root cause is
a schema/data-evolution bug from #20, not anything resource-related:

`RunRow.actorId/actorLabel/actorKind` were `Schema.NullOr(...)` —
present-but-nullable, which means **the key must exist**. Runs are
stored as JSON documents, and every run written *before* #20 has a
`data` doc with no such key. The store passes stored docs straight to
the HTTP response encoder, which rejects the newest row with `Missing
key at ["runs"][0]["actorId"]`. A response-encode failure emits a bare
400 (no body, no content-type) — the empty-body signature. Since no
executions have happened since the actor store deployed, 100% of stored
runs are legacy and every request fails.

## The fix

Make the three actor keys optional with a **decoding default of null**:

```ts
actorId: Schema.optional(Schema.NullOr(Schema.String)).pipe(
  Schema.withDecodingDefaultType(Effect.succeed(null)),
)
```

- **Decoded type stays `string | null`** (always present) — every reader
still treats the field as required; it just defaults to `null` for
pre-actor runs.
- **Wire/storage tolerates an absent key** — legacy docs decode *and*
encode unchanged.
- Immunizes the collection against the next field added the same way.

No data migration, no prod writes — deploy-forward and the page is back
with all run history intact.

## Verification

- New regression test round-trips a legacy-shaped doc (decode → null;
**encode with the keys absent succeeds** — the exact failure).
- execution-history suite 20/20, typecheck 41/41, lint + format clean.

(Build/Deploy-preview checks fail on the fork's missing R2 secrets, as
on prior PRs — unrelated.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant