Codex/result wrapping#16
Merged
Merged
Conversation
Convert the inner layers (leaf utils, adapters, core domain, and the
Store/Platform/LlmAdapter ports) from throwing to returning Result<T, Error>.
Errors propagate as values (`if (!x.ok) return x`) until a boundary frame —
the queue/scheduled handlers, Durable Object alarm/RPC methods, and the HTTP
route handlers — which are the only places that throw (runtime-critical, to
drive msg.retry()/DO retry/5xx) or map an Err to a handled response.
This replaces the earlier pass that wrapped calls in Result.from(...) and
immediately re-threw the error (a behavior-preserving no-op that defeated the
point of Result). Adds an `unwrap` helper for the loop-island pattern, since
asyncMap/asyncForEach/asyncUnfold cannot short-circuit on an Err and raw loops
are banned by local/no-raw-loops.
- Store (28 methods), Platform (10), LlmAdapter.complete -> Result<T, Error>
- 82 inner functions converted; domain invariants use Err(new Error("MSG"))
to keep the error channel uniformly Error (no string | Error unions)
- D1 reads are wrapped here (the queue/DO boundary needs read failures as
values to drive retry), consciously overriding the generic "don't wrap
reads" guidance for this port
- Fixes a type error in coordinator.ts where env-var narrowing was lost
inside a Result.from(() => ...) closure
typecheck, lint, and all tests pass.
- extract markDelivered helper (dedup KV puts + name DEDUPE_TTL_SECONDS) - multiline ternaries -> IIFEs; ?. chains -> guards; + concat -> template literal - adapter-llm: redundant Result.from+unwrap -> asyncForEach; return inner.complete directly - drop explicit return types on GitHubAdapter (let TS infer) - enforce Array<T> via @typescript-eslint/array-type; convert repo-wide
- Inner fns return Err; errors propagate as returned values through loop/asyncMap/asyncUnfold callbacks instead of throwing inside them (fail-slow on idempotent ops, accepted). - asyncUnfold infers its result as the union of STOP values (Step + Extract), removing per-call-site type hints. - Drop over-broad Result.from wrappers around asyncUnfold in adapter-github; only the smallest throwable IO stays wrapped.
- parseNativeId / parseSlackNativeId / d1-store json+parse return Result instead of throwing; callers unwrap and propagate to boundaries - configIdentitySource / safeParse / buildEngineContext / memberGate return Result; routes throw at the boundary, queue/cron propagate - add findMap helper; rewrite findIdentity over it - tests assert expect(x.ok) + x.data! instead of okOrThrow / throw - drop okOrThrow
Hoist the missing-secret check out of the verify ternary so a missing GITHUB_WEBHOOK_SECRET throws (500 server misconfig) instead of masking as a 401 bad signature. Test env now binds a dummy secret so the unsigned webhook test exercises real signature verification.
…ance resolveSlackUserId extracted Platform.resolvePerson into a bare variable and invoked it detached, so `this` was undefined and SlackAdapter's `this.config` threw a TypeError on every roster-username/email DM and digest resolution, crashing the unit of work. Call it as a method on `slack`. isOptionalString rejected explicit null, so a roster row with a null optional field (the natural JSON for an absent value) made configIdentitySource return Err and 500'd every webhook. Treat null as absent; still rejects non-strings. Both regressions were masked by the suite: route/aggregate mocked resolvePerson with this-free closures, and every roster fixture used the array path that skips row validation. Add a this-bound resolvePerson test and a null-field roster-string test; both fail without the fixes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.