fix(session) + test(regression): auth persistence + onboarding test suite (ENG-231, ENG-234)#44
Open
fix(session) + test(regression): auth persistence + onboarding test suite (ENG-231, ENG-234)#44
Conversation
TypeScript compiler does not copy non-.ts assets. PromptLoader resolves prompts from dist/prompts/ at runtime — without this copy step, the directory was missing after a clean build, causing prompt load failures. Fix: - Add scripts/copy-prompts.js: mirrors src/prompts/ → dist/prompts/ - Wire into build script: 'tsc && node scripts/copy-prompts.js' Result: single canonical dist/prompts/ path, no duplication, build verified clean (11 .md files copied across 3 subdirs). Closes #39 Resolves ENG-232
Wallet tools (CheckBalance, SendPayment, ReceivePayment, GetTransactionHistory, GetExchangeRate, EstimateFee) were throwing 'WalletPort not injected' errors because the adapter was never passed into the tool execution path. Changes: - ToolExecutionContext: add required walletPort: WalletPort field - AgentLoop: accept walletPort in constructor, thread into execContext - AgentOrchestrator: accept walletPort in constructor, pass to AgentLoop - app.module.ts + index.ts: wire FlashAPIAdapter into AgentOrchestrator - UserContext.IdentitySchema: add authToken field (set by VerifyOTP) - UserContext.PartialIdentityInput: expose authToken for test factories - VerifyOTP: persist authToken into identity context after successful OTP - All wallet tools: implement execute() using injected walletPort - CheckBalance: getBalance() with token registration - SendPayment: sendPayment() with idempotency key - ReceivePayment: createInvoice() with optional amount/expiry - GetTransactionHistory: getTransactionHistory() with filters - GetExchangeRate: getExchangeRate() (no auth required) - EstimateFee: estimateFee() with token registration - Tests: update ToolRegistry, AgentOrchestrator, CheckBalance test suites Result: all 496 tests pass, no direct Flash API calls in tool layer. Closes #40 Resolves ENG-233
Root causes: 1. ContextManager.saveContext() only updated memCache AFTER durable writes — if cold store threw (e.g. missing directory), memCache was never set, so the next turn loaded a new default context (accountLinked: false). 2. Memory cache TTL was 60s — too short for a multi-turn conversation. A slight delay between turns caused a cache miss + stale store read. 3. PersistentContextAdapter storage directory was never created at startup. The first save attempt failed with ENOENT, triggering the above. Fixes: - ContextManager.saveContext(): update memCache FIRST, before durable writes. Session stays alive in memory even if Redis/filesystem writes fail. Cold store errors still thrown (caller can log), but session is not lost. - Memory cache TTL: 60s → 1800s (30 minutes). Covers normal conversation gaps. - PersistentContextAdapter.initialize(): new method to mkdir the base path. Called at startup in both index.ts and app.module.ts before any I/O. Result: after successful OTP, check_balance works immediately on the next turn. accountLinked=true and authToken survive across all turns. Closes #38 Resolves ENG-231
…ENG-234) Adds 17 regression tests covering the P0 session persistence bug (pulse#38) and wallet adapter injection (pulse#40). Tests verify: - Unlinked user: wallet tools filtered out, get_account_status reports correctly - OTP success: patchContext preserves authToken + accountLinked after link - Balance check: check_balance calls walletPort with correct accountId and token - Session persistence: context survives save/load round-trips (3 cycles) - Repeat balance: second balance check uses same linked context (core regression) - Store resilience: memCache available even when cold store write fails All tests run in the standard jest suite (not integration/excluded path). No live API calls — all external I/O mocked via MockWalletPort, MockContextStore. Closes #41 Resolves ENG-234
patoo0x
changed the title
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.
Changes in this PR
ENG-231 — Session persistence fix
After OTP success, the next turn asked to re-link. Three root causes:
ContextManager.saveContext()updated memCache AFTER durable writes — cold store failure lost the sessionPersistentContextAdapternever created./data/contexts/at startup — first save = ENOENTFixes: memCache updated first, TTL → 30min,
initialize()method added + called at startup.ENG-234 — Regression test suite
17 tests covering the full onboarding → OTP → balance → repeat balance path.
Runs in standard jest suite — CI picks it up on every PR. All external I/O mocked.
Verification
Closes #38, Closes #41
Resolves ENG-231, ENG-234