Add permissioned partner execution pipeline - #22
Conversation
Migrate order creation to the V2 fixed-point contract, normalize immediate fill state, distinguish provider rejection from unknown outcomes, and bind authorization liquidity to persisted top-of-book snapshots. Quantize reservations to exact contract cost so accounting matches submitted orders.\n\nFocused execution tests, typecheck, Bun guard, glossary, and partner validation pass. The full suite retains pre-existing ops-server TDZ and Bun.TOML.stringify failures reproduced at base commit 920f7aa.
Record the V2 mapper and persisted-book snapshot loader as partial provider integration while keeping runtime route composition and reconciliation explicitly open. Domain test and TypeScript pass; full-suite baseline exceptions are documented in the parent commit.
Validation: 1131 tests pass; the sole full-suite failure is tests/partner/toml-config.test.ts because Bun 1.3.14 lacks Bun.TOML.stringify required by the repository's >=1.4.0-canary.1 runtime policy. Focused authorization/execution/HTTP tests (86), typecheck, Bun native guard, glossary, partner validators, TOML config validation, and root branded-ID check pass.
📝 WalkthroughWalkthroughThis PR adds a Bun 1.3.14 baseline, persisted authorization workflows, deterministic execution gates, exposure reservations, Kalshi V2 placement, Telegram approval commands, receipt delivery, live-order HTTP integration, and related tests and operator documentation. ChangesAuthorized execution
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5cd08ca to
686ce63
Compare
brendadeeznuts1111
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 842ac27c06
ℹ️ 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".
| if (!(req as Request & { compliance?: ComplianceContext }).compliance) { | ||
| return badOrder("E_AUTH_CONTEXT_REQUIRED", "live order did not pass compliance middleware"); |
There was a problem hiding this comment.
When a caller submits dryRun: false, this only verifies that some compliance context exists; it never checks that compliance.parsedBody.wagerAmount and marketId correspond to the parsed stakeMinorUnits and ticker. An API caller can therefore have compliance approve and record a small wager or different market while the authorized path executes a larger stake on another market. Reject the request unless the compliance terms exactly bind the order being executed.
AGENTS.md reference: AGENTS.md:L22-L24
Useful? React with 👍 / 👎.
| oddsFresh: | ||
| quote.fresh && | ||
| decimalOddsToKalshiPriceCents(request.decimalOdds) === quote.priceCents, |
There was a problem hiding this comment.
The quote's fresh flag is calculated before awaiting the balance, session, policy, and risk calls, then returned unchanged. If /portfolio/balance or another dependency takes longer than maxAgeMs, an already-expired quote still passes the gate and can produce a stale resting order rather than consuming the promised executable top of book. Recompute age at snapshot completion or again inside the reservation transaction.
AGENTS.md reference: AGENTS.md:L22-L24
Useful? React with 👍 / 👎.
| const lane = { | ||
| partnerCode: request.partnerCode, | ||
| outId: request.outId, | ||
| skin: request.skin, | ||
| }; | ||
| const outstandingExposure = computeOutstandingExposure(db, lane); |
There was a problem hiding this comment.
For an out with multiple active skins, the registry documents that those skins share one account balance, but this reservation lane includes skin. Two concurrent requests for different skins can both load the same pre-order Kalshi balance, and the second transaction will not see the first skin's placing reservation when subtracting outstandingExposure; both can therefore reserve the full shared balance and rely on the provider to reject or race the excess. Balance-bearing reservations need to aggregate at least at the out/account level, while any skin-specific policy exposure can remain separate.
AGENTS.md reference: AGENTS.md:L22-L24
Useful? React with 👍 / 👎.
| WHERE partner_code = $partnerCode | ||
| AND out_id = $outId | ||
| AND skin = $skin | ||
| AND market_id = $marketId | ||
| AND decimal_odds = $decimalOdds | ||
| AND status IN ('pending', 'placing', 'confirmed', 'unknown')`, |
There was a problem hiding this comment.
Executable Kalshi depth is shared across all partners, outs, and skins, but this query subtracts reservations only from the current lane. Concurrent orders from different lanes can each reserve the entire same persisted top-of-book level; after the first consumes it, later GTC orders may rest even though this route promises immediately executable liquidity. Aggregate concurrent reservations by market, selection, and quote without the partner/out/skin filters.
AGENTS.md reference: AGENTS.md:L22-L24
Useful? React with 👍 / 👎.
| if (result.execution?.code === "SNAPSHOT_UNAVAILABLE") { | ||
| return badOrder("E_UPSTREAM", result.reason); | ||
| } | ||
| return badOrder("E_AUTHORIZATION_REQUIRED", result.reason); |
There was a problem hiding this comment.
If Kalshi accepts an order but confirmReservation or receipt persistence fails, the executor returns PERSISTENCE_UNCERTAIN; this fallback converts it to HTTP 403 E_AUTHORIZATION_REQUIRED. A caller may interpret that as no placement and retry with a new idempotency key, duplicating an order that was actually accepted. Map PERSISTENCE_UNCERTAIN to the unknown-execution response so clients retain the original key and wait for reconciliation.
AGENTS.md reference: AGENTS.md:L22-L24
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (18)
tests/telegram/commands.test.ts (1)
21-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSplit the multiline-argument case out of the rejection test.
The test name states that the parser rejects malformed commands.
"/approve bad\narg"is not rejected; line 24 asserts that it parses into two arguments. The in-loopifmixes two behaviors under one name.Move the multiline case to its own test.
♻️ Proposed refactor
test("rejects malformed commands", () => { - for (const text of ["approve 1", "/", "/approve@ 1", "/approve bad\narg"]) { - if (text === "/approve bad\narg") { - expect(parseTelegramCommand(text)?.args).toEqual(["bad", "arg"]); - } else { - expect(parseTelegramCommand(text)).toBeNull(); - } - } + for (const text of ["approve 1", "/", "/approve@ 1", "/1approve"]) { + expect(parseTelegramCommand(text)).toBeNull(); + } + }); + + test("splits arguments on any whitespace, including newlines", () => { + expect(parseTelegramCommand("/approve bad\narg")?.args).toEqual(["bad", "arg"]); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/telegram/commands.test.ts` around lines 21 - 29, Split the "/approve bad\narg" case from the "rejects malformed commands" test in parseTelegramCommand tests. Keep only rejected inputs in the existing loop, and add a separate clearly named test that asserts the multiline command parses to ["bad", "arg"].src/telegram/bot.ts (1)
216-226: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSeparate receipt delivery and maintenance from the
getUpdatesfailure path.
getUpdatesat line 208 shares the try block withdeliverAuthorizationReceiptBatchandrunExecutionMaintenance. WhengetUpdatesthrows, both later steps are skipped for that iteration. Pending receipts stay undelivered and stale exposure reservations stay held during a Telegram polling outage.Wrap the update fetch in its own try block so the outbox and maintenance steps still run.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/telegram/bot.ts` around lines 216 - 226, Separate the getUpdates call from the receipt and maintenance workflow by placing only the update fetch and its handling in its own try/catch. Ensure deliverAuthorizationReceiptBatch and runExecutionMaintenance execute for every polling iteration even when getUpdates fails, while preserving their existing order and behavior.src/telegram/authorization-commands.ts (2)
209-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the current-policy resolution.
The nested
??and=== undefinedtest encodes three cases in one expression. The behavior is correct, but this is a compliance gate. An explicit branch makes the fail-closed intent easier to audit.♻️ Proposed refactor
- const currentPolicy = - dependencies.resolveCurrentPolicy?.(request) ?? - (dependencies.resolveCurrentPolicy === undefined - ? getCurrentAuthorizationPolicy(dependencies.db, request) - : null); + const currentPolicy = + dependencies.resolveCurrentPolicy === undefined + ? getCurrentAuthorizationPolicy(dependencies.db, request) + : dependencies.resolveCurrentPolicy(request);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/telegram/authorization-commands.ts` around lines 209 - 213, Replace the nested currentPolicy expression in the authorization command with an explicit branch: use dependencies.resolveCurrentPolicy when provided, otherwise call getCurrentAuthorizationPolicy, while preserving null/failed-resolution behavior as the fail-closed result. Keep the existing request, database, and policy-resolution semantics unchanged.
169-176: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLog the database error before you return
COMMAND_DATABASE_ERROR.The catch block discards the error. The caller receives only a code, and
handleCommandinsrc/telegram/bot.tsreturns without any output. An operator then has no record of why a permissioned/approveor/revoke_outcommand failed.Capture the error and log it. Do not include the receipt text in the log, because it contains partner and out identifiers.
♻️ Proposed change
- } catch { + } catch (error) { + console.error("Authorization command transaction failed", { + command: parsed.name, + error: error instanceof Error ? error.message : String(error), + }); return { handled: true, ok: false, code: "COMMAND_DATABASE_ERROR", receiptOutboxId: null, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/telegram/authorization-commands.ts` around lines 169 - 176, Update the catch block in the command authorization flow to capture the thrown database error and log it before returning COMMAND_DATABASE_ERROR. Include only safe error context and exclude receipt text or partner/out identifiers; preserve the existing handled, ok, code, and receiptOutboxId response.tests/partner/execution/executor.test.ts (1)
146-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this test, and add coverage for the unresolved
placingstate.The test awaits each call in sequence, so it verifies exposure accounting across calls, not serialization. Rename it to describe accumulated exposure, or dispatch both calls with
Promise.allto exercise theBEGIN IMMEDIATEpath.Also add a test where the finalize transaction fails after the provider accepts. That path returns
PERSISTENCE_UNCERTAINand leaves the row inplacing, which is the recovery gap noted insrc/partner/execution/executor.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/partner/execution/executor.test.ts` around lines 146 - 166, Rename the sequential test around executeAuthorizedBet to describe accumulated exposure, or dispatch both executions concurrently with Promise.all to verify reservation serialization. Add coverage for a provider-accepted bet whose finalize transaction fails, asserting executeAuthorizedBet returns PERSISTENCE_UNCERTAIN and leaves the reservation row in placing for recovery.src/partner/execution/maintenance.ts (1)
16-23: 🚀 Performance & Scalability | 🔵 TrivialPlan for table growth in the maintenance tick.
This aggregate scans every row of
exposure_reservations. Confirmed and settled rows stay in the table, so each tick becomes more expensive over time. Add an index onstatus, or restrict the count to unresolved rows and archive terminal rows on a retention schedule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/partner/execution/maintenance.ts` around lines 16 - 23, Update the maintenance tick’s exposure reservation count query near the counts aggregate to avoid scanning all historical rows: add and use an index on exposure_reservations.status, or restrict counting to unresolved statuses and implement retention-based archiving for terminal rows. Preserve the placing and unknown counts while ensuring confirmed and settled records do not cause unbounded per-tick work.src/partner/execution/domain.ts (1)
24-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo sources of truth for the reservation lifecycle statuses. The status set is declared once as a TypeScript constant and again as a SQL CHECK literal list. A status added in one file does not reach the other, so inserts fail at runtime instead of at type check.
src/partner/execution/domain.ts#L24-L32: keepEXPOSURE_RESERVATION_STATUSESas the single source of truth and export a helper that renders the SQLIN (...)list from it.src/partner/execution/sql.ts#L24-L26: build thestatusCHECK from that helper instead of repeating the seven literals.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/partner/execution/domain.ts` around lines 24 - 32, The reservation status list is duplicated between TypeScript and SQL. In src/partner/execution/domain.ts lines 24-32, keep EXPOSURE_RESERVATION_STATUSES as the source of truth and export a helper that renders its values as a SQL IN-list; in src/partner/execution/sql.ts lines 24-26, replace the repeated literals in the status CHECK constraint with that helper.src/partner/execution/executor.ts (1)
54-58: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMove schema migration off the order request path.
ensureExecutionSchemarunsmigrateAuthorizationSchema, aCREATE TABLE IF NOT EXISTS, and a migration-table query on every live order. Those are write-path statements that can contend with the reservation transaction under load.Run the migration once at startup, and keep the executor read-only with respect to schema.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/partner/execution/executor.ts` around lines 54 - 58, Move the ensureExecutionSchema call out of the live order execution flow and invoke it once during application startup before requests are accepted. Remove the try/catch schema-migration block from the executor so the reservation path remains read-only with respect to schema, while preserving existing startup failure handling.src/partner/execution/reservation.ts (1)
455-471: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the interpolated status clause with a fixed predicate.
statusSqlis an internal literal today, so no injection exists. The template interpolation still triggers the SQL-injection rules from ast-grep and OpenGrep on every scan.sumExposurehas one caller with one status set, so the parameter is unused generality.Inline the status list in
computeOutstandingExposureand delete thestatusSqlparameter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/partner/execution/reservation.ts` around lines 455 - 471, Remove the statusSql parameter and interpolation from sumExposure, then inline the required fixed status predicate in its SQL query. Update computeOutstandingExposure, the sole caller, to use the revised sumExposure signature while preserving the existing status set.Source: Linters/SAST tools
src/partner/execution/kalshi-snapshot.ts (1)
218-222: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAvoid spread over unbounded parsed arrays.
book_ticks.levels_jsonis external data with no level-count limit.Math.max(...array)andMath.min(...array)can exceed the argument limit and throwRangeErrorfor very large books. Usereduceinstead.♻️ Proposed refactor
function isCrossed(book: BookSnapshot): boolean { - const bestBid = Math.max(...book.bids.map((level) => level.priceCents), 0); - const bestAsk = Math.min(...book.asks.map((level) => level.priceCents), 100); + const bestBid = book.bids.reduce((max, level) => Math.max(max, level.priceCents), 0); + const bestAsk = book.asks.reduce((min, level) => Math.min(min, level.priceCents), 100); return bestBid > 0 && bestAsk < 100 && bestBid > bestAsk; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/partner/execution/kalshi-snapshot.ts` around lines 218 - 222, Update isCrossed to compute bestBid and bestAsk with reduce rather than spreading the mapped price arrays into Math.max or Math.min, while preserving the existing 0 and 100 initial bounds and crossing conditions.tests/research/trading-order.test.ts (1)
56-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the
attachCompliancehelper.Lines 60-65 repeat the body of
attachCompliancedefined at line 167. Call the helper instead.♻️ Proposed refactor
const req = request(liveBody(), { "Idempotency-Key": "live-http-1" }); - (req as Request & { compliance?: unknown }).compliance = { - stateCode: "MA", - userId: "operator-1", - playId: "play-1", - parsedBody: {}, - }; + attachCompliance(req);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/research/trading-order.test.ts` around lines 56 - 65, Replace the inline compliance assignment in the “Fantasy402 is explicitly 501 and never reaches provider placement” test with a call to the existing attachCompliance helper. Pass the request and the same compliance values so the test behavior remains unchanged, and avoid duplicating the helper’s setup logic.tests/partner/execution/kalshi-live.test.ts (1)
62-105: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for the
nooutcome.Every execution test uses
outcome: "yes", so the derived NO price path inbestBuyLevel(100 - bestYesBid) stays untested end to end. Add one case withoutcome: "no"and a matchingpriceCentsof 65 for the fixture book, and assert the placed order side and count.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/partner/execution/kalshi-live.test.ts` around lines 62 - 105, Add a separate execution test alongside the existing test using outcome "no" and the fixture book’s derived priceCents of 65. Assert that executeKalshiLiveOrder places one order with side "no" and the expected count, covering the bestBuyLevel NO-price path end to end.src/research/hq-view.ts (1)
361-403: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDeduplicate the live-order client logic.
submitOrderhere duplicatessrc/research/hq-app/app.jslines 711-779, including the required-field list, the header set, and the payload keys. Two copies must change together for every contract change. Extract one shared script module and load it from both entry points.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/research/hq-view.ts` around lines 361 - 403, Extract the duplicated live-order submission logic from submitOrder and the corresponding flow in hq-app/app.js into one shared script module, including required-field validation, headers, and payload construction. Update both entry points to load and use the shared module, preserving their existing UI behavior and avoiding separate contract definitions.AGENTS.md (1)
3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the machine-specific inherited-instruction path.
Lines 3-5 make repository instructions depend on
/Users/nolarose/Projects/AGENTS.md, which does not exist in other clones. Keep required authority in tracked repository files. Treat external DX guidance as optional and unable to override this file.Proposed change
-This standalone repository inherits `/Users/nolarose/Projects/AGENTS.md` and -the global DX context. This file narrows their application; it does not weaken -runtime safety. +This file defines the repository-local authority. Optional external DX guidance +may add workflow context, but it cannot weaken runtime safety.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AGENTS.md` around lines 3 - 5, Update the repository guidance in AGENTS.md to remove the machine-specific /Users/nolarose/Projects/AGENTS.md inheritance reference. Keep required instructions authoritative within tracked repository files, and state that any external DX guidance is optional and cannot override this file.src/partner/authorization/outbox.ts (1)
197-238: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHand-written transaction control is not nest-safe across the authorization persistence layer. Three functions issue
BEGIN IMMEDIATE,COMMIT, andROLLBACKdirectly. If any caller already holds a transaction,BEGINthrows and thecatchthen rolls back the caller's outer work.service.tsalready usesdb.transaction(...).immediate(), which nests through savepoints; adopt that convention here.
src/partner/authorization/outbox.ts#L197-L238: wrap the candidate select and the claim loop ofclaimDueAuthorizationReceiptsindb.transaction(...).immediate()and delete the manualBEGIN,COMMIT, andROLLBACKcalls.src/partner/authorization/outbox.ts#L283-L339: wrap the lease check and the status update ofmarkAuthorizationReceiptFailedindb.transaction(...).immediate(), and returnnullfrom inside the transaction body instead of issuing the earlyCOMMITat Line 299.src/partner/authorization/sql.ts#L234-L249: wrap each migration body ofmigrateAuthorizationSchemaindb.transaction(...).immediate(), then push the migration ID after the transaction returns.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/partner/authorization/outbox.ts` around lines 197 - 238, Replace manual transaction control in claimDueAuthorizationReceipts (src/partner/authorization/outbox.ts:197-238) with db.transaction(...).immediate(), enclosing the candidate selection and claim loop. Do the same for markAuthorizationReceiptFailed (src/partner/authorization/outbox.ts:283-339), returning null inside the transaction body instead of committing early. In migrateAuthorizationSchema (src/partner/authorization/sql.ts:234-249), wrap each migration body with db.transaction(...).immediate() and push its migration ID only after the transaction returns; remove direct BEGIN, COMMIT, and ROLLBACK usage at all three sites.src/partner/authorization/service.ts (1)
241-243: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDo not return raw database error text to callers.
databaseFailurereturnsError.messageverbatim. That message reaches Telegram replies and HTTP responses through thereasonfield, and SQLite messages include table, column, and constraint names. Log the detail and return a stable reason.🛡️ Proposed change
-function databaseFailure(reason: unknown): string { - return reason instanceof Error ? reason.message : "authorization database operation failed"; -} +function databaseFailure(reason: unknown): string { + if (reason instanceof Error) { + console.error("[authorization] database operation failed", reason); + } + return "authorization database operation failed"; +}Keep a separate validation formatter if the input-validation messages must stay user-visible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/partner/authorization/service.ts` around lines 241 - 243, Update databaseFailure to log the underlying Error detail internally while always returning the stable authorization database failure reason to callers. Preserve any separate validation formatter used for user-visible input-validation messages, and ensure database errors propagated through Telegram or HTTP reason fields never expose raw database text.src/bot/kalshi-client.ts (1)
214-224: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the dry-run result builder.
The same dry-run object literal exists at Line 214 and at Line 315 in the module-level
placeOrder. Extract one helper so the two paths cannot drift.♻️ Proposed helper
+function dryRunResult(request: KalshiOrderRequest): KalshiOrderResult { + return { + orderId: `dry-${request.ticker}-${Date.now()}`, + clientOrderId: request.clientOrderId ?? crypto.randomUUID(), + fillCount: 0, + remainingCount: request.count, + averageFillPriceCents: null, + averageFeePaidCents: null, + processedAtMs: null, + dryRun: true, + }; +}Then both call sites return
dryRunResult(request).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bot/kalshi-client.ts` around lines 214 - 224, Extract the duplicated dry-run order object construction into a shared module-level helper, such as dryRunResult, preserving all fields and their current values. Update both dry-run paths, including placeOrder and the other visible call site, to return the helper result so the implementations cannot diverge.tests/partner/authorization/service.test.ts (1)
117-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThrow instead of returning on the negative branch.
if (!result.ok) return;narrows the type, but it also ends the test body without running the remaining assertions. A regression then produces a passing test. ThecreateRequesthelper at Line 84 already throws; use the same pattern here and at Line 187.♻️ Proposed change
expect(result.ok).toBeTrue(); - if (!result.ok) return; + if (!result.ok) throw new Error(result.reason);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/partner/authorization/service.test.ts` around lines 117 - 118, In the authorization tests, replace the early return after the negative `result.ok` assertion with a throwing failure, matching the existing `createRequest` helper pattern. Apply the same change to the corresponding branch near the second referenced assertion so test execution cannot end successfully before remaining assertions run.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bot/kalshi-client.ts`:
- Around line 329-331: Update cancelOrder to use the default client’s V2
order-cancellation method and endpoint, matching the V2 create-order flow,
instead of the legacy portfolio orders mutation. Preserve the existing orderId
argument and Promise<void> behavior.
In `@src/partner/authorization/service.ts`:
- Around line 333-357: Align getCurrentAuthorizationPolicy with its documented
approval-policy meaning by excluding pending requests from the selected source,
or revise the comment to accurately describe pending-request behavior. Ensure
approveAuthorizationRequest does not treat a newer pending row as the current
approved policy while preserving the existing newest-row ordering.
In `@src/partner/execution/executor.ts`:
- Around line 232-240: Add a reservation helper near the existing reservation
state transitions to update expired placing rows to unknown, preserving any
existing failure_reason and recording the maintenance timestamp. Invoke this
helper from runExecutionMaintenance and include the number of swept reservations
in its maintenance reporting so existing unknown reconciliation can process
them.
In `@src/partner/execution/kalshi-live.ts`:
- Around line 215-219: After the existing client guard, assign the narrowed
client to a const and update loadBalance to call getBalance on that const
instead of the let client variable. Keep the balancePromise memoization behavior
unchanged.
In `@src/partner/execution/reservation.ts`:
- Around line 253-278: Update computeReservedMarketLiquidity to accept a
selection parameter and add selection filtering to its exposure_reservations
query. Pass request.selection from executeAuthorizedBet when calling it,
preserving the existing market, odds, lane, and status filters so reservations
are aggregated only for the quoted side.
In `@src/partner/execution/sql.ts`:
- Line 15: Enable SQLite foreign-key enforcement immediately after each Database
instance is created, covering all connection-opening paths found via new
Database calls, rather than relying on migrateExecutionSchema. Ensure every
connection used to write execution data enforces the authorization_id reference,
while retaining the existing migration behavior as needed.
In `@src/partner/toml-stringify.ts`:
- Around line 152-156: Update tomlStringify and its related tests or consumers
so native TOML serialization and fallbackTomlStringify produce equivalent
canonical output, particularly scalar-before-table ordering; where exact text
equivalence is not required, replace serialized-text comparisons with
parse-and-compare assertions.
In `@src/research/serve.ts`:
- Around line 266-302: The live-order safety gate must require the production
arming flag. In src/research/serve.ts lines 266-302, update handleTradingOrder’s
default isRiskHealthy logic so KALSHI_AUTHORIZED_EXECUTION_ENABLED=1 remains
required and KALSHI_PROD_ARMED=1 is additionally required when KALSHI_ENV is
prod. In src/partner/execution/kalshi-live.ts lines 293-302, update
createKalshiAccountClientResolver to reject prod client construction unless
envMap.KALSHI_PROD_ARMED equals "1", covering all callers.
In `@src/telegram/api.ts`:
- Around line 56-84: Add a request timeout to the fetch call in sendMessage by
passing Bun’s AbortSignal.timeout through the request options. Use the
established Telegram timeout configuration if one exists; otherwise define an
appropriate bounded timeout, while preserving the existing response parsing and
error behavior.
In `@src/telegram/authorization-commands.ts`:
- Around line 169-176: Update src/telegram/authorization-commands.ts lines
169-176 in the permissioned /approve and /revoke_out command error handling to
bind the caught error and log it with the command name, without logging receipt
text; update src/telegram/bot.ts lines 72-79 before the early return to detect a
null authorization.receiptOutboxId, log the failure code, and send a direct
failure notice to the chat.
In `@src/telegram/authorization-outbox-worker.ts`:
- Around line 57-64: Prevent duplicate receipt delivery by treating lease loss
after a successful send as non-fatal: in the authorization outbox worker’s send
flow around markAuthorizationReceiptSent, record the lease loss and increment
sent instead of throwing into the retry path. Also update
asAuthorizationReceiptLeaseOwner in src/telegram/bot.ts (lines 200-202) to
include the hostname and a random suffix alongside the process identity,
ensuring lease owners are unique across hosts.
In `@tests/telegram/api.test.ts`:
- Around line 14-17: Await the rejection assertion in the test “can be imported
without a token and fails only when called” so the test waits for sendMessage’s
promise and verifies the TELEGRAM_BOT_TOKEN error before cleanup runs.
In `@tests/telegram/authorization-commands.test.ts`:
- Around line 183-193: Extend the replay-safety test around
handleAuthorizationCommand to assert that account_authorization_revocations
contains exactly one row after the repeated revoke request. Keep the existing
row-content assertion and outbox count assertion unchanged.
- Around line 97-101: Update the assertion flow in the authorization receipt
test: explicitly assert that first.receiptOutboxId is not null before retrieving
the outbox item, then perform the receipt-text assertion unconditionally using
that validated ID. Remove the conditional guard around the existing
getAuthorizationReceiptOutboxItem check.
In `@tests/telegram/authorization-requests.test.ts`:
- Around line 83-111: Update the test around postAuthorizationRequest to assert
the branch-specific failure code: expect TELEGRAM_SEND_FAILED when the send
callback throws and TELEGRAM_RESPONSE_MISMATCH when it returns a different
topic, while preserving the existing persistence assertions.
In `@tools/telegram/setup-alert-hub.ts`:
- Around line 74-79: Update the command menu definition in setup-alert-hub.ts to
include entries for both status and help, matching the commands handled by
handleCommand and advertised by the existing /start and /help text. Preserve the
current command descriptions and ordering unless needed to add these missing
entries.
---
Nitpick comments:
In `@AGENTS.md`:
- Around line 3-5: Update the repository guidance in AGENTS.md to remove the
machine-specific /Users/nolarose/Projects/AGENTS.md inheritance reference. Keep
required instructions authoritative within tracked repository files, and state
that any external DX guidance is optional and cannot override this file.
In `@src/bot/kalshi-client.ts`:
- Around line 214-224: Extract the duplicated dry-run order object construction
into a shared module-level helper, such as dryRunResult, preserving all fields
and their current values. Update both dry-run paths, including placeOrder and
the other visible call site, to return the helper result so the implementations
cannot diverge.
In `@src/partner/authorization/outbox.ts`:
- Around line 197-238: Replace manual transaction control in
claimDueAuthorizationReceipts (src/partner/authorization/outbox.ts:197-238) with
db.transaction(...).immediate(), enclosing the candidate selection and claim
loop. Do the same for markAuthorizationReceiptFailed
(src/partner/authorization/outbox.ts:283-339), returning null inside the
transaction body instead of committing early. In migrateAuthorizationSchema
(src/partner/authorization/sql.ts:234-249), wrap each migration body with
db.transaction(...).immediate() and push its migration ID only after the
transaction returns; remove direct BEGIN, COMMIT, and ROLLBACK usage at all
three sites.
In `@src/partner/authorization/service.ts`:
- Around line 241-243: Update databaseFailure to log the underlying Error detail
internally while always returning the stable authorization database failure
reason to callers. Preserve any separate validation formatter used for
user-visible input-validation messages, and ensure database errors propagated
through Telegram or HTTP reason fields never expose raw database text.
In `@src/partner/execution/domain.ts`:
- Around line 24-32: The reservation status list is duplicated between
TypeScript and SQL. In src/partner/execution/domain.ts lines 24-32, keep
EXPOSURE_RESERVATION_STATUSES as the source of truth and export a helper that
renders its values as a SQL IN-list; in src/partner/execution/sql.ts lines
24-26, replace the repeated literals in the status CHECK constraint with that
helper.
In `@src/partner/execution/executor.ts`:
- Around line 54-58: Move the ensureExecutionSchema call out of the live order
execution flow and invoke it once during application startup before requests are
accepted. Remove the try/catch schema-migration block from the executor so the
reservation path remains read-only with respect to schema, while preserving
existing startup failure handling.
In `@src/partner/execution/kalshi-snapshot.ts`:
- Around line 218-222: Update isCrossed to compute bestBid and bestAsk with
reduce rather than spreading the mapped price arrays into Math.max or Math.min,
while preserving the existing 0 and 100 initial bounds and crossing conditions.
In `@src/partner/execution/maintenance.ts`:
- Around line 16-23: Update the maintenance tick’s exposure reservation count
query near the counts aggregate to avoid scanning all historical rows: add and
use an index on exposure_reservations.status, or restrict counting to unresolved
statuses and implement retention-based archiving for terminal rows. Preserve the
placing and unknown counts while ensuring confirmed and settled records do not
cause unbounded per-tick work.
In `@src/partner/execution/reservation.ts`:
- Around line 455-471: Remove the statusSql parameter and interpolation from
sumExposure, then inline the required fixed status predicate in its SQL query.
Update computeOutstandingExposure, the sole caller, to use the revised
sumExposure signature while preserving the existing status set.
In `@src/research/hq-view.ts`:
- Around line 361-403: Extract the duplicated live-order submission logic from
submitOrder and the corresponding flow in hq-app/app.js into one shared script
module, including required-field validation, headers, and payload construction.
Update both entry points to load and use the shared module, preserving their
existing UI behavior and avoiding separate contract definitions.
In `@src/telegram/authorization-commands.ts`:
- Around line 209-213: Replace the nested currentPolicy expression in the
authorization command with an explicit branch: use
dependencies.resolveCurrentPolicy when provided, otherwise call
getCurrentAuthorizationPolicy, while preserving null/failed-resolution behavior
as the fail-closed result. Keep the existing request, database, and
policy-resolution semantics unchanged.
- Around line 169-176: Update the catch block in the command authorization flow
to capture the thrown database error and log it before returning
COMMAND_DATABASE_ERROR. Include only safe error context and exclude receipt text
or partner/out identifiers; preserve the existing handled, ok, code, and
receiptOutboxId response.
In `@src/telegram/bot.ts`:
- Around line 216-226: Separate the getUpdates call from the receipt and
maintenance workflow by placing only the update fetch and its handling in its
own try/catch. Ensure deliverAuthorizationReceiptBatch and
runExecutionMaintenance execute for every polling iteration even when getUpdates
fails, while preserving their existing order and behavior.
In `@tests/partner/authorization/service.test.ts`:
- Around line 117-118: In the authorization tests, replace the early return
after the negative `result.ok` assertion with a throwing failure, matching the
existing `createRequest` helper pattern. Apply the same change to the
corresponding branch near the second referenced assertion so test execution
cannot end successfully before remaining assertions run.
In `@tests/partner/execution/executor.test.ts`:
- Around line 146-166: Rename the sequential test around executeAuthorizedBet to
describe accumulated exposure, or dispatch both executions concurrently with
Promise.all to verify reservation serialization. Add coverage for a
provider-accepted bet whose finalize transaction fails, asserting
executeAuthorizedBet returns PERSISTENCE_UNCERTAIN and leaves the reservation
row in placing for recovery.
In `@tests/partner/execution/kalshi-live.test.ts`:
- Around line 62-105: Add a separate execution test alongside the existing test
using outcome "no" and the fixture book’s derived priceCents of 65. Assert that
executeKalshiLiveOrder places one order with side "no" and the expected count,
covering the bestBuyLevel NO-price path end to end.
In `@tests/research/trading-order.test.ts`:
- Around line 56-65: Replace the inline compliance assignment in the “Fantasy402
is explicitly 501 and never reaches provider placement” test with a call to the
existing attachCompliance helper. Pass the request and the same compliance
values so the test behavior remains unchanged, and avoid duplicating the
helper’s setup logic.
In `@tests/telegram/commands.test.ts`:
- Around line 21-29: Split the "/approve bad\narg" case from the "rejects
malformed commands" test in parseTelegramCommand tests. Keep only rejected
inputs in the existing loop, and add a separate clearly named test that asserts
the multiline command parses to ["bad", "arg"].
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b50ba25-6fed-428f-b9ca-d87200bbe62f
📒 Files selected for processing (62)
.bun-version.env.example.github/pull_request_template.md.github/workflows/check.ymlAGENTS.mdREADME.mdbunfig.tomldocs/AUTHORIZED_EXECUTION.mddocs/ENV_NAMING.mdpackage.jsonsrc/bot/kalshi-client.tssrc/institutions/error-codes.tssrc/lib/config.tssrc/partner/authorization/domain.tssrc/partner/authorization/gate.tssrc/partner/authorization/hash.tssrc/partner/authorization/index.tssrc/partner/authorization/outbox.tssrc/partner/authorization/service.tssrc/partner/authorization/sql.tssrc/partner/authorization/stake.tssrc/partner/domain.tssrc/partner/execution/domain.tssrc/partner/execution/executor.tssrc/partner/execution/index.tssrc/partner/execution/kalshi-live.tssrc/partner/execution/kalshi-snapshot.tssrc/partner/execution/kalshi.tssrc/partner/execution/maintenance.tssrc/partner/execution/reservation.tssrc/partner/execution/sql.tssrc/partner/index.tssrc/partner/toml-stringify.tssrc/research/hq-app/app.jssrc/research/hq-view.tssrc/research/serve.tssrc/telegram/api.tssrc/telegram/authorization-commands.tssrc/telegram/authorization-outbox-worker.tssrc/telegram/authorization-requests.tssrc/telegram/bot.tssrc/telegram/commands.tstests/bot/kalshi-client.test.tstests/partner/authorization/gate.test.tstests/partner/authorization/hash.test.tstests/partner/authorization/outbox.test.tstests/partner/authorization/service.test.tstests/partner/authorization/sql.test.tstests/partner/authorization/stake.test.tstests/partner/domain.test.tstests/partner/execution/executor.test.tstests/partner/execution/kalshi-live.test.tstests/partner/execution/kalshi-snapshot.test.tstests/partner/execution/kalshi.test.tstests/partner/execution/reservation.test.tstests/research/trading-order.test.tstests/telegram/api.test.tstests/telegram/authorization-commands.test.tstests/telegram/authorization-outbox-worker.test.tstests/telegram/authorization-requests.test.tstests/telegram/commands.test.tstools/telegram/setup-alert-hub.ts
| export async function cancelOrder(orderId: string): Promise<void> { | ||
| return getDefaultClient().cancelOrder(orderId); | ||
| return getDefaultKalshiClient().cancelOrder(orderId); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Kalshi API DELETE /portfolio/events/orders cancel order V2 legacy /portfolio/orders deprecation date
💡 Result:
The legacy /portfolio/orders mutation endpoints, including DELETE /portfolio/orders/{order_id}, were scheduled for deprecation in June 2026 [1][2]. According to Kalshi's official changelog, these endpoints were marked for deprecation between June 18 and June 25, 2026 [1][2]. Upon deprecation, calls to these legacy paths return a response directing users to "Please switch to the V2 endpoints" [1][2]. For canceling orders, developers must now use the V2 endpoint [3][4]: DELETE /portfolio/events/orders/{order_id} Key differences for the V2 Cancel Order endpoint include: - It uses a V2 response shape, returning an object containing {order_id, client_order_id, reduced_by} instead of a full order object [3][4]. - It is part of the broader V2 /portfolio/events/orders/ family, which requires clients to migrate from legacy /portfolio/orders/ paths [1][2][5]. As of August 6, 2026, integration with the legacy /portfolio/orders endpoints should be considered deprecated, and all new or existing order management operations should target the V2 event-order endpoints [1][2].
Citations:
- 1: https://docs.kalshi.com/fix/changelog
- 2: https://docs.kalshi.com/changelog/index.md
- 3: https://docs.kalshi.com/api-reference/orders/cancel-order-v2
- 4: https://unpkg.com/kalshi-typescript@3.25.0/docs/OrdersApi.md
- 5: https://docs.kalshi.com/api-reference/orders/create-order-v2
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant files:\n'
fd -a 'kalshi-client|order|portfolio' . | sed 's#^\./##' | head -80
printf '\nkalshi-client outline:\n'
ast-grep outline src/bot/kalshi-client.ts --view expanded || true
printf '\nRelevant kalshi-client sections:\n'
wc -l src/bot/kalshi-client.ts
sed -n '260,360p' src/bot/kalshi-client.ts
sed -n '1,140p' src/bot/kalshi-client.ts
sed -n '360,520p' src/bot/kalshi-client.ts
printf '\nSearch order endpoints in repository:\n'
rg -n "portfolio/events/orders|portfolio/orders|cancelOrder|createOrder|cancel-" --glob '*.{ts,tsx}' .Repository: brendadeeznuts1111/Kalshi-bot
Length of output: 1944
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files available:\n'
git ls-files | sed -n '1,120p'
printf '\nLocate Kalshi client and order symbols:\n'
fd -a 'kalshi-client|order|portfolio' . | sed 's#^\./##' | sed -n '1,120p'
printf '\nRelevant kalshi-client outline:\n'
ast-grep outline src/bot/KalshiClient.ts --view expanded 2>/dev/null ||
ast-grep outline src/bot/kalshi-client.ts --view expanded 2>/dev/null || true
printf '\nRelevant kalshi-client sections:\n'
for f in src/bot/kalshi-client.ts src/bot/KalshiClient.ts; do
[ -f "$f" ] || continue
echo "=== $f ==="
wc -l "$f"
sed -n '1,140p' "$f"
sed -n '260,380p' "$f"
sed -n '380,540p' "$f"
done
printf '\nSearch portfolio order endpoints in repository:\n'
rg -n "portfolio/events/orders|portfolio/orders|cancelOrder|createOrder|cancel-" --glob '*.{ts,tsx}' . || trueRepository: brendadeeznuts1111/Kalshi-bot
Length of output: 1944
Migrate cancelOrder to the V2 order endpoint.
cancelOrder still calls the legacy DELETE /portfolio/orders/{orderId} route. Kalshi has deprecated the legacy /portfolio/orders mutations and routes V2 order cancels through DELETE /portfolio/events/orders/{id}, so align the cancel path with the V2 create path to avoid using a stale endpoint.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bot/kalshi-client.ts` around lines 329 - 331, Update cancelOrder to use
the default client’s V2 order-cancellation method and endpoint, matching the V2
create-order flow, instead of the legacy portfolio orders mutation. Preserve the
existing orderId argument and Promise<void> behavior.
| /** Resolve the newest approval policy snapshot for the same partner/out/provider/skin lane. */ | ||
| export function getCurrentAuthorizationPolicy( | ||
| db: Database, | ||
| request: AuthorizationRequest, | ||
| ): AuthorizationPolicy | null { | ||
| const row = db | ||
| .query( | ||
| `SELECT * | ||
| FROM account_authorization_requests | ||
| WHERE partner_code = $partnerCode | ||
| AND out_id = $outId | ||
| AND provider = $provider | ||
| AND skin = $skin | ||
| AND status IN ('pending', 'approved') | ||
| ORDER BY created_at_ms DESC, id DESC | ||
| LIMIT 1`, | ||
| ) | ||
| .get({ | ||
| $partnerCode: request.partnerCode, | ||
| $outId: request.outId, | ||
| $provider: request.provider, | ||
| $skin: request.skin, | ||
| }) as RequestRow | null; | ||
| return row === null ? null : requestPolicy(row); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The current-policy source does not match its documented meaning.
The comment states "approval policy snapshot", but the query reads account_authorization_requests and accepts pending rows. A newer pending request for the same partner, out, provider, and skin therefore becomes the "current policy". approveAuthorizationRequest then compares that policy against the older request hash and denies with POLICY_HASH_MISMATCH. The outcome stays fail-closed, so this is a contract clarity defect, not an unsafe grant. Restrict the query to the approval source you intend, or correct the comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/partner/authorization/service.ts` around lines 333 - 357, Align
getCurrentAuthorizationPolicy with its documented approval-policy meaning by
excluding pending requests from the selected source, or revise the comment to
accurately describe pending-request behavior. Ensure approveAuthorizationRequest
does not treat a newer pending row as the current approved policy while
preserving the existing newest-row ordering.
| const claimed = claimReservationForPlacement(db, { | ||
| id: created.reservation.id, | ||
| placementOwner, | ||
| nowMs, | ||
| }); | ||
| if (claimed === null) throw new Error("new reservation could not be claimed for placement"); | ||
| return { kind: "place" as const, authorization, reservation: claimed }; | ||
| }); | ||
| prepared = transaction.immediate(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Add a recovery path for stale placing reservations.
The reservation becomes placing inside the transaction, and the provider call runs after the transaction commits. If the process stops, or if a finalize transaction fails and the code returns PERSISTENCE_UNCERTAIN, the row stays placing forever:
releaseExpiredReservationsupdates onlypendingrows.runExecutionMaintenancecountsplacingrows and never changes them.reconcileUnknownAsConfirmedandreconcileUnknownAsRejectedaccept onlyunknownrows.
The stake stays inside computeOutstandingExposure and computeDailyUsage, so the lane loses that exposure budget with no operator action available. Add a transition from expired placing to unknown, so the existing reconciliation functions can resolve the row against the provider by idempotency key.
🛠️ Proposed reservation helper to unblock reconciliation
// src/partner/execution/reservation.ts
/** Move dispatch-owned rows that outlived their TTL into `unknown` for reconciliation. */
export function markExpiredPlacingAsUnknown(db: Database, nowMs = Date.now()): number {
assertTimestamp(nowMs, "placing expiry sweep time");
return db
.query(
`UPDATE exposure_reservations
SET status = 'unknown',
failure_reason = COALESCE(failure_reason, 'placement outcome never recorded'),
updated_at_ms = $nowMs
WHERE status = 'placing' AND reservation_expires_at_ms <= $nowMs`,
)
.run({ $nowMs: nowMs }).changes;
}Call it from runExecutionMaintenance and report the swept count.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/partner/execution/executor.ts` around lines 232 - 240, Add a reservation
helper near the existing reservation state transitions to update expired placing
rows to unknown, preserving any existing failure_reason and recording the
maintenance timestamp. Invoke this helper from runExecutionMaintenance and
include the number of swept reservations in its maintenance reporting so
existing unknown reconciliation can process them.
| let balancePromise: ReturnType<typeof client.getBalance> | null = null; | ||
| const loadBalance = () => { | ||
| balancePromise ??= client.getBalance(); | ||
| return balancePromise; | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Capture client in a const before the closure.
client is declared with let and typed ... | undefined. TypeScript discards the if (!client) return narrowing inside the arrow function at line 217, so client.getBalance() can fail the type check under strictNullChecks. Assign a narrowed const after the guard.
♻️ Proposed fix
+ const provider = client;
let balancePromise: ReturnType<typeof client.getBalance> | null = null;
const loadBalance = () => {
- balancePromise ??= client.getBalance();
+ balancePromise ??= provider.getBalance();
return balancePromise;
};#!/bin/bash
# Confirm strict null checks are enabled for this file.
fd -H -t f 'tsconfig*.json' | xargs -I{} sh -c 'echo "== {}"; cat {}'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/partner/execution/kalshi-live.ts` around lines 215 - 219, After the
existing client guard, assign the narrowed client to a const and update
loadBalance to call getBalance on that const instead of the let client variable.
Keep the balancePromise memoization behavior unchanged.
| export function computeReservedMarketLiquidity( | ||
| db: Database, | ||
| lane: ReservationLane, | ||
| marketId: BetRequest["marketId"], | ||
| decimalOdds: number, | ||
| ): number { | ||
| const row = db | ||
| .query( | ||
| `SELECT COALESCE(SUM(effective_stake), 0) AS total | ||
| FROM exposure_reservations | ||
| WHERE partner_code = $partnerCode | ||
| AND out_id = $outId | ||
| AND skin = $skin | ||
| AND market_id = $marketId | ||
| AND decimal_odds = $decimalOdds | ||
| AND status IN ('pending', 'placing', 'confirmed', 'unknown')`, | ||
| ) | ||
| .get({ | ||
| $partnerCode: lane.partnerCode, | ||
| $outId: lane.outId, | ||
| $skin: lane.skin, | ||
| $marketId: marketId, | ||
| $decimalOdds: decimalOdds, | ||
| }) as { total: number }; | ||
| return assertAggregate(row.total); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Filter reserved market liquidity by selection.
The snapshot loader returns side-specific depth. loadKalshiMarketExecutionQuote reports YES depth from the ask level and NO depth from the bid level. The executor subtracts computeReservedMarketLiquidity from that side-specific value, but this query aggregates every selection in the market.
If both sides quote the same decimal_odds, for example 2.0 on a 50/50 book, an opposite-side reservation consumes the wrong side's depth. Migration 002 already adds the (market_id, selection, status) index for this access path.
🐛 Proposed fix to scope the aggregate to one selection
export function computeReservedMarketLiquidity(
db: Database,
lane: ReservationLane,
marketId: BetRequest["marketId"],
decimalOdds: number,
+ selection: BetRequest["selection"],
): number {
const row = db
.query(
`SELECT COALESCE(SUM(effective_stake), 0) AS total
FROM exposure_reservations
WHERE partner_code = $partnerCode
AND out_id = $outId
AND skin = $skin
AND market_id = $marketId
+ AND selection = $selection
AND decimal_odds = $decimalOdds
AND status IN ('pending', 'placing', 'confirmed', 'unknown')`,
)
.get({
$partnerCode: lane.partnerCode,
$outId: lane.outId,
$skin: lane.skin,
$marketId: marketId,
+ $selection: selection,
$decimalOdds: decimalOdds,
}) as { total: number };
return assertAggregate(row.total);
}Pass request.selection from executeAuthorizedBet in src/partner/execution/executor.ts (lines 159-164).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/partner/execution/reservation.ts` around lines 253 - 278, Update
computeReservedMarketLiquidity to accept a selection parameter and add selection
filtering to its exposure_reservations query. Pass request.selection from
executeAuthorizedBet when calling it, preserving the existing market, odds,
lane, and status filters so reservations are aggregated only for the quoted
side.
| test("can be imported without a token and fails only when called", async () => { | ||
| delete Bun.env.TELEGRAM_BOT_TOKEN; | ||
| expect(sendMessage(-123, "hello")).rejects.toThrow("TELEGRAM_BOT_TOKEN not set"); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Await the rejection assertion.
expect(...).rejects.toThrow(...) returns a promise. Line 16 does not await it. The test resolves before the assertion settles, so it passes even when sendMessage does not throw. The token-absence guard is therefore not verified. The floating promise can also reject after afterEach restores the token.
💚 Proposed fix
delete Bun.env.TELEGRAM_BOT_TOKEN;
- expect(sendMessage(-123, "hello")).rejects.toThrow("TELEGRAM_BOT_TOKEN not set");
+ await expect(sendMessage(-123, "hello")).rejects.toThrow("TELEGRAM_BOT_TOKEN not set");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test("can be imported without a token and fails only when called", async () => { | |
| delete Bun.env.TELEGRAM_BOT_TOKEN; | |
| expect(sendMessage(-123, "hello")).rejects.toThrow("TELEGRAM_BOT_TOKEN not set"); | |
| }); | |
| test("can be imported without a token and fails only when called", async () => { | |
| delete Bun.env.TELEGRAM_BOT_TOKEN; | |
| await expect(sendMessage(-123, "hello")).rejects.toThrow("TELEGRAM_BOT_TOKEN not set"); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/telegram/api.test.ts` around lines 14 - 17, Await the rejection
assertion in the test “can be imported without a token and fails only when
called” so the test waits for sendMessage’s promise and verifies the
TELEGRAM_BOT_TOKEN error before cleanup runs.
| if (first.handled && first.receiptOutboxId !== null) { | ||
| expect(getAuthorizationReceiptOutboxItem(db, first.receiptOutboxId)?.payload.text).toContain( | ||
| "Authorization active", | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that receiptOutboxId is not null.
The if guard makes the receipt-text assertion conditional. If receiptOutboxId regresses to null, the block does not run and the test still passes. The toMatchObject at line 89 does not cover the id.
Assert the id is present, then read the item.
💚 Proposed fix
- if (first.handled && first.receiptOutboxId !== null) {
- expect(getAuthorizationReceiptOutboxItem(db, first.receiptOutboxId)?.payload.text).toContain(
- "Authorization active",
- );
- }
+ expect(first.handled && first.receiptOutboxId).not.toBeNull();
+ if (!first.handled || first.receiptOutboxId === null) throw new Error("receipt not queued");
+ expect(getAuthorizationReceiptOutboxItem(db, first.receiptOutboxId)?.payload.text).toContain(
+ "Authorization active",
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (first.handled && first.receiptOutboxId !== null) { | |
| expect(getAuthorizationReceiptOutboxItem(db, first.receiptOutboxId)?.payload.text).toContain( | |
| "Authorization active", | |
| ); | |
| } | |
| expect(first.handled && first.receiptOutboxId).not.toBeNull(); | |
| if (!first.handled || first.receiptOutboxId === null) throw new Error("receipt not queued"); | |
| expect(getAuthorizationReceiptOutboxItem(db, first.receiptOutboxId)?.payload.text).toContain( | |
| "Authorization active", | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/telegram/authorization-commands.test.ts` around lines 97 - 101, Update
the assertion flow in the authorization receipt test: explicitly assert that
first.receiptOutboxId is not null before retrieving the outbox item, then
perform the receipt-text assertion unconditionally using that validated ID.
Remove the conditional guard around the existing
getAuthorizationReceiptOutboxItem check.
| const revokeMessage = message("/revoke_out out-SPORTS-1", { message_id: 201 }); | ||
| const first = handleAuthorizationCommand({ db }, revokeMessage, NOW_MS + 2); | ||
| const replay = handleAuthorizationCommand({ db }, revokeMessage, NOW_MS + 3); | ||
| expect(first).toMatchObject({ handled: true, ok: true, code: "OUT_AUTHORIZATIONS_REVOKED" }); | ||
| expect(replay).toMatchObject({ handled: true, ok: true, code: "OUT_AUTHORIZATIONS_REVOKED" }); | ||
| expect( | ||
| db.query("SELECT out_id, telegram_message_id FROM account_authorization_revocations").get(), | ||
| ).toEqual({ out_id: "out-SPORTS-1", telegram_message_id: "201" }); | ||
| expect( | ||
| db.query("SELECT count(*) AS count FROM account_authorization_receipt_outbox").get(), | ||
| ).toEqual({ count: 2 }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the revocation row count to prove replay safety.
The test name states replay safety. Line 188-190 uses .get(), which returns only the first row. A replay that inserts a second row in account_authorization_revocations would still satisfy this assertion. The outbox count of 2 covers the approve receipt and the revoke receipt, not revocation-row duplication.
Add a count assertion.
💚 Proposed addition
expect(
db.query("SELECT out_id, telegram_message_id FROM account_authorization_revocations").get(),
).toEqual({ out_id: "out-SPORTS-1", telegram_message_id: "201" });
+ expect(
+ db.query("SELECT count(*) AS count FROM account_authorization_revocations").get(),
+ ).toEqual({ count: 1 });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const revokeMessage = message("/revoke_out out-SPORTS-1", { message_id: 201 }); | |
| const first = handleAuthorizationCommand({ db }, revokeMessage, NOW_MS + 2); | |
| const replay = handleAuthorizationCommand({ db }, revokeMessage, NOW_MS + 3); | |
| expect(first).toMatchObject({ handled: true, ok: true, code: "OUT_AUTHORIZATIONS_REVOKED" }); | |
| expect(replay).toMatchObject({ handled: true, ok: true, code: "OUT_AUTHORIZATIONS_REVOKED" }); | |
| expect( | |
| db.query("SELECT out_id, telegram_message_id FROM account_authorization_revocations").get(), | |
| ).toEqual({ out_id: "out-SPORTS-1", telegram_message_id: "201" }); | |
| expect( | |
| db.query("SELECT count(*) AS count FROM account_authorization_receipt_outbox").get(), | |
| ).toEqual({ count: 2 }); | |
| const revokeMessage = message("/revoke_out out-SPORTS-1", { message_id: 201 }); | |
| const first = handleAuthorizationCommand({ db }, revokeMessage, NOW_MS + 2); | |
| const replay = handleAuthorizationCommand({ db }, revokeMessage, NOW_MS + 3); | |
| expect(first).toMatchObject({ handled: true, ok: true, code: "OUT_AUTHORIZATIONS_REVOKED" }); | |
| expect(replay).toMatchObject({ handled: true, ok: true, code: "OUT_AUTHORIZATIONS_REVOKED" }); | |
| expect( | |
| db.query("SELECT out_id, telegram_message_id FROM account_authorization_revocations").get(), | |
| ).toEqual({ out_id: "out-SPORTS-1", telegram_message_id: "201" }); | |
| expect( | |
| db.query("SELECT count(*) AS count FROM account_authorization_revocations").get(), | |
| ).toEqual({ count: 1 }); | |
| expect( | |
| db.query("SELECT count(*) AS count FROM account_authorization_receipt_outbox").get(), | |
| ).toEqual({ count: 2 }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/telegram/authorization-commands.test.ts` around lines 183 - 193, Extend
the replay-safety test around handleAuthorizationCommand to assert that
account_authorization_revocations contains exactly one row after the repeated
revoke request. Keep the existing row-content assertion and outbox count
assertion unchanged.
| test("fails without persistence when Telegram send fails or returns another topic", async () => { | ||
| for (const mismatch of [false, true]) { | ||
| const db = database(); | ||
| const result = await postAuthorizationRequest( | ||
| db, | ||
| { | ||
| policy: policy(), | ||
| telegramChatId: asTelegramChatId("-123"), | ||
| telegramTopicId: asTelegramTopicId("7"), | ||
| nowMs: NOW_MS, | ||
| }, | ||
| async (_chatId, text) => { | ||
| if (!mismatch) throw new Error("offline"); | ||
| return { | ||
| message_id: 100, | ||
| message_thread_id: 8, | ||
| chat: { id: -123, type: "supergroup" }, | ||
| date: Math.floor(NOW_MS / 1_000), | ||
| text, | ||
| }; | ||
| }, | ||
| ); | ||
| expect(result.ok).toBeFalse(); | ||
| expect( | ||
| db.query("SELECT count(*) AS count FROM account_authorization_requests").get(), | ||
| ).toEqual({ count: 0 }); | ||
| db.close(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the specific failure code for each branch.
Line 105 checks only that result.ok is false. The two branches must produce different codes: TELEGRAM_SEND_FAILED when the send throws, and TELEGRAM_RESPONSE_MISMATCH when Telegram returns another topic. A regression that collapses both into one code still passes this test.
💚 Proposed fix
- expect(result.ok).toBeFalse();
+ expect(result).toMatchObject({
+ ok: false,
+ code: mismatch ? "TELEGRAM_RESPONSE_MISMATCH" : "TELEGRAM_SEND_FAILED",
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test("fails without persistence when Telegram send fails or returns another topic", async () => { | |
| for (const mismatch of [false, true]) { | |
| const db = database(); | |
| const result = await postAuthorizationRequest( | |
| db, | |
| { | |
| policy: policy(), | |
| telegramChatId: asTelegramChatId("-123"), | |
| telegramTopicId: asTelegramTopicId("7"), | |
| nowMs: NOW_MS, | |
| }, | |
| async (_chatId, text) => { | |
| if (!mismatch) throw new Error("offline"); | |
| return { | |
| message_id: 100, | |
| message_thread_id: 8, | |
| chat: { id: -123, type: "supergroup" }, | |
| date: Math.floor(NOW_MS / 1_000), | |
| text, | |
| }; | |
| }, | |
| ); | |
| expect(result.ok).toBeFalse(); | |
| expect( | |
| db.query("SELECT count(*) AS count FROM account_authorization_requests").get(), | |
| ).toEqual({ count: 0 }); | |
| db.close(); | |
| } | |
| }); | |
| test("fails without persistence when Telegram send fails or returns another topic", async () => { | |
| for (const mismatch of [false, true]) { | |
| const db = database(); | |
| const result = await postAuthorizationRequest( | |
| db, | |
| { | |
| policy: policy(), | |
| telegramChatId: asTelegramChatId("-123"), | |
| telegramTopicId: asTelegramTopicId("7"), | |
| nowMs: NOW_MS, | |
| }, | |
| async (_chatId, text) => { | |
| if (!mismatch) throw new Error("offline"); | |
| return { | |
| message_id: 100, | |
| message_thread_id: 8, | |
| chat: { id: -123, type: "supergroup" }, | |
| date: Math.floor(NOW_MS / 1_000), | |
| text, | |
| }; | |
| }, | |
| ); | |
| expect(result).toMatchObject({ | |
| ok: false, | |
| code: mismatch ? "TELEGRAM_RESPONSE_MISMATCH" : "TELEGRAM_SEND_FAILED", | |
| }); | |
| expect( | |
| db.query("SELECT count(*) AS count FROM account_authorization_requests").get(), | |
| ).toEqual({ count: 0 }); | |
| db.close(); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/telegram/authorization-requests.test.ts` around lines 83 - 111, Update
the test around postAuthorizationRequest to assert the branch-specific failure
code: expect TELEGRAM_SEND_FAILED when the send callback throws and
TELEGRAM_RESPONSE_MISMATCH when it returns a different topic, while preserving
the existing persistence assertions.
| { command: "dashboard", description: "Latest calibration dashboard" }, | ||
| { command: "members", description: "Channel member count and admins" }, | ||
| { command: "dash", description: "Link to ops dashboard" }, | ||
| { command: "subscribe", description: "Subscribe this chat to the digest" }, | ||
| { command: "unsubscribe", description: "Remove this chat from the digest" }, | ||
| { command: "approve", description: "Approve an authorization request by ID" }, | ||
| { command: "revoke_out", description: "Revoke active grants for an out" }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the status and help entries to the command menu.
handleCommand in src/telegram/bot.ts handles status and help. The /start and /help texts also advertise /status. This list omits both, so they do not appear in the Telegram command menu.
♻️ Proposed addition
{ command: "dashboard", description: "Latest calibration dashboard" },
+ { command: "status", description: "Live program metrics" },
{ command: "members", description: "Channel member count and admins" },
{ command: "subscribe", description: "Subscribe this chat to the digest" },
{ command: "unsubscribe", description: "Remove this chat from the digest" },
{ command: "approve", description: "Approve an authorization request by ID" },
{ command: "revoke_out", description: "Revoke active grants for an out" },
+ { command: "help", description: "Command reference" },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { command: "dashboard", description: "Latest calibration dashboard" }, | |
| { command: "members", description: "Channel member count and admins" }, | |
| { command: "dash", description: "Link to ops dashboard" }, | |
| { command: "subscribe", description: "Subscribe this chat to the digest" }, | |
| { command: "unsubscribe", description: "Remove this chat from the digest" }, | |
| { command: "approve", description: "Approve an authorization request by ID" }, | |
| { command: "revoke_out", description: "Revoke active grants for an out" }, | |
| { command: "dashboard", description: "Latest calibration dashboard" }, | |
| { command: "status", description: "Live program metrics" }, | |
| { command: "members", description: "Channel member count and admins" }, | |
| { command: "subscribe", description: "Subscribe this chat to the digest" }, | |
| { command: "unsubscribe", description: "Remove this chat from the digest" }, | |
| { command: "approve", description: "Approve an authorization request by ID" }, | |
| { command: "revoke_out", description: "Revoke active grants for an out" }, | |
| { command: "help", description: "Command reference" }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/telegram/setup-alert-hub.ts` around lines 74 - 79, Update the command
menu definition in setup-alert-hub.ts to include entries for both status and
help, matching the commands handled by handleCommand and advertised by the
existing /start and /help text. Preserve the current command descriptions and
ordering unless needed to add these missing entries.
1 participant
Outcome
Adds a permissioned, fail-closed partner execution pipeline from Telegram approval through audited Kalshi placement. Governance cleanup now makes Bun 1.3.14 local proof authoritative, removes billing-blocked hosted checks from automatic PR status, and documents exactly which flags and state can enable live execution.
What changed
POST /api/trading/orderthrough compliance, authorization, balance, liquidity, reservation, and provider boundariesbun run bun:cithe local merge proofSafety and compatibility
KALSHI_AUTHORIZED_EXECUTION_ENABLED=1KALSHI_ENV=prodandKALSHI_PROD_ARMED=1Validation
bun run bun:ci: 1,121 pass, 0 failbun run typecheckbun run guardbun run glossary:checkbun run partners:validatebun run check:brandsdx package: ready on Bun/package-manager 1.3.14workflow_dispatchFollow-up
Summary by CodeRabbit