feat: add two-path financial management (invoices, email, QB writes)#651
Merged
feat: add two-path financial management (invoices, email, QB writes)#651
Conversation
Implements three phases of the Two-Path Financial Management plan:
Phase 1 - Invoice Model and Local Invoice Generation:
- Invoice and InvoiceLineItem ORM models, Alembic migration (002)
- InvoiceStore with CRUD operations and globally unique ID generation
- Invoice PDF generation via ReportLab
- generate_invoice and convert_estimate_to_invoice agent tools
- GET /invoices/{id}/pdf API endpoint
- 23 tests covering tools, store, PDF, and API
Phase 2 - Email Sending Service:
- Abstract EmailService with Resend (httpx) and SMTP (aiosmtplib) implementations
- get_email_service() factory based on EMAIL_PROVIDER config
- send_document_email agent tool with PDF attachment support
- Auto-updates document status to "sent" on successful delivery
- Email config settings (provider, from address, API keys, SMTP params)
- 22 tests covering both service implementations and the tool
Phase 3 - QuickBooks Write Operations and Auto-Switching:
- create_entity() and send_invoice_email() on QuickBooksService ABC
- 5 new QB tools: qb_create_estimate, qb_create_invoice,
qb_create_customer, qb_send_invoice, qb_estimate_to_invoice
- Auto-disable local tools (estimate, invoice, email) when QB is connected
- auto_disabled_reason field on tool config API and frontend
- Frontend shows auto-disabled state with amber text and locked toggle
- 24 tests covering QB write operations and auto-switching logic
Also includes prerequisite database migration work (PostgreSQL-backed
stores replacing file-based stores) that was needed for the invoice
and email features.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…on test) - Fix 110 type errors: update tests to use User ORM model instead of UserData DTO - Fix docker-compose healthcheck: CMD-EXEC -> CMD-SHELL - Remove unused @testing-library/user-event dependency - Fix integration test: create BOOTSTRAP.md before asserting is_onboarding_needed - Split User.__init__ defaults to avoid call-top-callable type error Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ding The file-to-Postgres migration moved user persistence to the DB but dropped the filesystem provisioning (SOUL.md, USER.md, HEARTBEAT.md, BOOTSTRAP.md) that the old file_store._save() handled. Without BOOTSTRAP.md, is_onboarding_needed() returns False and onboarding is immediately marked complete for new users. Add provision_user_directory() and call it from both user creation paths (get_current_user and _get_or_create_user). Add regression tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Workspace tools now route reads/writes for USER.md, SOUL.md, and HEARTBEAT.md through the User DB row (user_text, soul_text, heartbeat_text columns) instead of the filesystem. This eliminates the split-brain where the agent wrote to disk but the profile API and system prompt read from the DB. - workspace_tools: DB-backed virtual files for the three protected .md files, disk I/O for everything else (BOOTSTRAP.md, memory/*, etc.) - provision_user (renamed from provision_user_directory): seeds default templates into DB columns instead of disk files - onboarding heuristics: read from user.user_text/soul_text instead of disk; refresh from DB before checking after agent runs - Updated all affected tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use Numeric(12, 2) instead of Float for all monetary columns - Fix client_id defaults (None instead of empty string for nullable FK) - Add ondelete CASCADE/SET NULL on all foreign keys - Add updated_at columns to clients, estimates, invoices - Fix race conditions with with_for_update() in ID generation - Use SQL-level concatenation in memory append to prevent lost updates - Add path traversal prevention in email, invoice, and workspace tools - Add XML escaping for ReportLab Paragraph() content - Add entity allowlist in QuickBooks query tool - Add input validation (email pattern, invoice ID format, date pattern) - Fix QB auto-disable to check token validity, not just connection status - Remove filesystem paths from tool responses - Fix heartbeat dual-source by always rebuilding from DB items first - Fix non-atomic user provisioning in ingestion - Fix TypeScript ID type mismatches (number -> string) - Fix tests to match updated behavior - Fix lint issues (unused imports, f-string, import order, ClassVar) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add ondelete="SET NULL" on Invoice.estimate_id FK (migration + model) - Harden QB customer name lookup: strip control chars before escaping - Wrap PDF generation in try/except for graceful ToolResult errors - Fix session ID race condition with UUID suffix + IntegrityError retry - Reject auto-disabled tool enable attempts with 400 instead of silent ignore - Add db_session() context manager with explicit rollback on error - Convert multi-step write methods to use db_session() across all stores - Fix search.py type annotation (UserData -> User) - Update webchat session_id regex to accept new UUID-suffixed format - Add user-scoping test for email tools Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix EstimateStore._next_estimate_number() to query globally (not
per-user) preventing PK collisions between concurrent users
- Add with_for_update() to ClientStore.create() slug query to prevent
race conditions on concurrent client creation
- Add user_id filter to SessionStore.load_session() preventing
cross-user session access
- Fix IdempotencyStore.mark_seen() TOCTOU race with IntegrityError
handling via db_session()
- Fix Client.estimates/invoices cascade conflict: remove delete-orphan
cascade to match ondelete=SET NULL FK behavior
- Add index=True to Invoice.estimate_id in ORM to match migration
- Add ondelete=CASCADE to all user_id FKs in ORM to match migrations
- Switch all store write methods from bare SessionLocal() to
db_session() for proper rollback on commit failure
- Add path containment check on invoice PDF write path
- Add try/except around invoice_store.create for graceful error handling
- Remove dead _DATE_RE regex from invoice_tools
- Add email validation pattern to QBSendInvoiceParams
- Add FROM clause requirement in QB query validation
- Add error handling around QB token refresh persistence
- Add email validation in QuickBooksService.send_invoice_email
- Make workspace_tools _db_read/_db_write async via asyncio.to_thread
- Accept 2xx status codes for Resend API (not just 200)
- Use MIMEMultipart("alternative") for SMTP when both text and HTML
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add path traversal prevention in estimate_tools PDF generation - Sanitize estimate ID in qb_estimate_to_invoice before query interpolation - Fix MediaStore.update() to use db_session() context manager for proper rollback on error (consistent with all other store write methods) - Filter _next_estimate_number/_next_invoice_number to only lock matching rows (EST-*/INV-*) instead of all rows, reducing lock contention - Add thread-safe double-checked locking to get_engine() and get_session_factory() singletons in database.py Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract shared QB auto-disable logic into get_qb_auto_disabled_groups() used by both agent router and tool config API, fixing mismatch where agent could disable local tools even with broken QB tokens - Add SMTP header injection prevention: sanitize To, Subject, From headers by stripping \r\n and angle brackets from from_name - Add field allowlists to all store update methods (UserStore, EstimateStore, InvoiceStore, HeartbeatStore, MediaStore, SessionStore) preventing unrestricted setattr that could allow ownership hijacking - Handle IntegrityError in _get_or_create_user for concurrent user creation races (common with Telegram webhook retries) - Change Mapped[float] to Mapped[Decimal] for all Numeric monetary columns (total_amount, quantity, unit_price, total, cost) to preserve precision - Add ondelete=CASCADE to Message.session_id FK in ORM to match migration - Add passive_deletes=True on Client.estimates/invoices relationships - Switch session_db update_message/update_compaction_seq to db_session() context manager for proper rollback on commit failure - Add user_id scoping to add_message/update_message/update_compaction_seq ChatSession lookups in session_db - Remove filesystem path leak from estimate tool result - Add try/except around estimate_store.create and invoice_store.create in convert_estimate_to_invoice for graceful error handling - Fix httpx.AsyncClient leak in QuickBooksOnlineService by using async context manager per-request instead of persistent client - Add pattern validators to QB date fields (expiration_date, due_date) - Update tests to match new mock paths and result format Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Lock the parent ChatSession row instead of using with_for_update() on the MAX(seq) aggregate query. PostgreSQL rejects FOR UPDATE with aggregate functions; SQLite silently accepts it, which is why the existing test suite did not catch this. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace in-memory SQLite with a real PostgreSQL database (clawbolt_test) for all tests. Uses transaction rollback with join_transaction_mode for per-test isolation, so tests remain fast and independent. This caught and fixed several real bugs that SQLite was hiding: - FK violations in estimate/invoice tools (client_id set without creating the Client record first) - Type mismatches (integer IDs passed to VARCHAR columns) - Unpersisted User objects referenced by child records Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CI test/integration/e2e jobs were failing because the test suite now requires PostgreSQL but no service container was configured. Add postgres:16-alpine service containers with health checks to all three test jobs. Also fix 8 type errors in client_db.py where Decimal values from Numeric ORM columns were passed directly to float-typed DTO fields. Wrap with float() at the ORM-to-DTO boundary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
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.
Description
Implements the Two-Path Financial Management plan across three phases, enabling Clawbolt to support both standalone local financial management and QuickBooks-connected workflows.
Phase 1: Invoice Model and Local Invoice Generation
generate_invoiceandconvert_estimate_to_invoiceagent toolsGET /invoices/{id}/pdfAPI endpointPhase 2: Email Sending Service
EmailServiceABC with Resend (httpx) and SMTP (aiosmtplib) implementationsget_email_service()factory driven byEMAIL_PROVIDERconfigsend_document_emailagent tool: sends estimate/invoice PDFs as email attachmentsPhase 3: QuickBooks Write Operations and Auto-Switching
create_entity()andsend_invoice_email()methods on QuickBooksServiceqb_create_estimate,qb_create_invoice,qb_create_customer,qb_send_invoice,qb_estimate_to_invoiceauto_disabled_reasonfield on tool config API responseAlso includes prerequisite PostgreSQL-backed store migration (replacing file-based stores) needed for invoice and email features.
Type
Checklist
uv run pytest -v) -- 1083 passedruff check backend/ && ruff format --check backend/)AI Usage
🤖 Generated with Claude Code