fix: prevent duplicate user messages in DB Agent LLM context #3738
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=80&v=4){kind=small}
Resolves route06/liam-internal#5820 Previously, user messages were included twice in the LLM context: 1. As part of the contextPrompt in invokeDesignAgent 2. In the messages array passed from setupWorkflowState This fix sets messages to an empty array when creating dbAgentState, following the same pattern used in createGraph.ts for the deep modeling flow. This ensures the user input is only included once via the prompt field. Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Unexpected error on running flow
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Updates to Preview Branch (devin/1760076304-fix-db-agent-message-duplication) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
Check changeset necessityStatus: NOT REQUIRED Reason:
Changeset (copy & paste):No changeset required — this PR only affects @liam-hq/agent (ignored package). |
🤖 Agent Deep Modeling ExecutionProcessing time: 5m 21s Command Output
RUN v3.2.4 /home/runner/work/liam/liam/frontend/internal-packages/agent (node:7777) ExperimentalWarning: WASI is an experimental feature and might change at any time ✅ [INFO] 2025-10-10T06:12:00.755Z 🧠 Thinking (pm)... Creating database requirements I'm taking on the role of PM Agent to create a Business Requirements Document (BRD) and structured requirements. My goal is to save analyzed requirements at the end, including test cases. Since there’s no current schema provided, I need to craft requirements that will help the Database Agent design a database, filling in gaps with standard assumptions. I’ll provide a concise checklist and review the user’s input about designing a simple user management system. I must translate this into clear business and functional requirements while balancing complexity and simplicity, including key elements like roles, permissions, and security features. Organizing test cases for requirements I'm focusing on creating test cases organized into categories, using titles and specific SQL operation types. These test cases will be included within the saveRequirementsToArtifactTool. For the BRD, I’ll categorize them into areas like User Account Management, Authentication, Roles, Data Integrity, Auditing, and APIs. It’s important to ensure these test cases support QA in verifying database design, covering operations such as insert, select, update, and delete for constraints and role assignments. I'll make sure each test case is clearly titled, positive and negative, and user-focused. Avoiding heavy formatting in documentation I need to keep formatting light based on the system instructions, which say that while bullet lists are fine, heavy formatting should be avoided. I’ll structure the document with clear labels and bullet points where appropriate. When I use the saveRequirementsToArtifactTool, I’ll need to clearly state the purpose and minimal inputs in one line as part of the workflow. For saving, the purpose will be “Save analyzed requirements and test cases,” and I’ll include the relevant inputs before confirming successful completion. The structure will also include the Business Requirements Document (BRD). Structuring the requirements document I’m planning to include several key sections in the document. First, I’ll outline the overview and goals, along with what's in-scope and out-of-scope. Then, I'll define personas or roles like Administrator, Standard User, and possibly Support. Next, I’ll draft high-level use cases and group functional requirements into categories such as User Accounts, Authentication, Roles, and more. Additionally, I'll cover data requirements, non-functional requirements like performance and compliance, and list assumptions. Lastly, I'll summarize acceptance criteria while keeping test cases separate, include a data dictionary for the DB Agent, and consider edge cases, but I’ll keep everything concise. Designing test case categories and data models I’m planning to create several test case categories like User Registration & Verification, Login & Session, Roles & Permissions, and more. Each category will include 2-6 test cases focused on user scenarios, aiming for a total of around 20-30 concise test cases overall. Next, I’ll structure the data model with entities such as users, roles, and user_roles, defining constraints like unique emails and optional properties. I'll also consider session management and audit logs but will keep the design simple. Lastly, I’ll detail functional flows for user management, covering everything from account creation to password resets and user roles. After that, I’ll prepare the BRD with all necessary sections before structuring the requirements into test cases using the saveRequirementsToArtifactTool. Defining test cases and rules I’m mapping out test cases covering various categories for user management. In Registration & Verification, I'll test for scenarios like creating users with pending status and rejecting duplicate emails. In Authentication & Sessions, I'll ensure successful logins create session records and that suspended users can't log in. For Password Management, I want to enforce password policies and manage token usage. In Roles & Permissions, I need to handle role assignments, ensuring no duplicate roles and keeping at least one role assigned. For Profile Management, user updates are key, and I’ll also address account lifecycle situations, like soft deletes. I need to enforce data integrity when it comes to unique email constraints and log auditing actions accurately. Test cases must ensure there are clear rules around all paths! Developing user management test cases I'm breaking down the test cases into categories for user management. In Reporting & Search, I’m including actions like listing active users by role and searching users by partial email. Under Security/Privacy, I note that tests must reflect SQL operations types. I need to create an initial checklist and build out the Business Requirements Document (BRD) with concise bullet points. Then, I’ll summarize the goal for the tool call. My focus is on clarity and simplicity, so I want to cover all relevant test cases without overwhelming detail. Let’s keep this thorough yet user-friendly! ⏺ (pm): Planned actions:
Business Requirements Document (BRD): Simple User Management System
Out of Scope
FR2 Email Verification
FR3 Authentication and Sessions
FR4 Password Management
FR5 Roles and Permissions
FR6 Profile Management
FR7 Account Status & Lifecycle
FR8 Auditing
FR9 Search & Reporting
Test Scenarios (for QA; also saved as artifact)
Preparing to save requirements artifact: purpose=Save goal and structured testcases for DB/QA; inputs=goal string and categorized testcases object. 🔧 saveRequirementsToArtifactTool() ⎿ unknown: Requirements saved successfully to artifact 🧠 Thinking (db)... Evaluating audit log constraints I'm thinking about the audit logs and the immutable constraint. It seems like I can't easily enforce the no update or delete rules. I wonder if I could include a trigger instead, but that doesn't seem possible. I realize I can at least mark some columns as notNull, so that's something. I want to make sure I proceed correctly with this, weighing the limitations carefully. It feels like there’s a lot to consider here! 🔧 schemaDesignTool() 🧠 Thinking (db)... Modifying database schema I'm planning to modify the audit_logs table by changing the id type to "bigserial" and taking steps to avoid creation sequence errors. I'll also make sure to manage existing indexes. The operations I'll call include removing the invalid index for users' email, adding a normalized email column, and creating a unique index on that column. Additionally, I’ll ensure the id type is adjusted correctly and remove any previously failed index creation paths. Finally, I’ll validate and report the results. 🔧 schemaDesignTool() 🧠 Thinking (db)... Designing schema operations I need to call the schema design tool right away since I have to design the schema. There were errors before due to an empty state, so I want to provide operations for creating enums and tables. I'll craft these operations carefully, ensuring to set valid defaults. It's better to add an email_normalized column with default null since the app expects a value. However, it can't default to lower(email), but setting notNull to false might contradict the requirement for a unique, non-null email. Setting up email constraints I need to enforce that the email field is unique and not null, aiming for case-insensitive checks via the application. I considered creating a functional index using "lower(email)," but it didn't work due to parentheses. Instead, I’ll create an email_normalized column that's not null with a default value, but this breaks uniqueness. To simplify, I’ll create the email column as text, marked not null, and use the "citext" type for case insensitivity, which requires adding the extension. I'll also update audit_logs id to bigserial and prepare operations for various tables and constraints, ensuring no expression index exists. 🔧 schemaDesignTool() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() role, a user, and assign ⏺ (qa): { ⏺ (qa): { 🔧 saveTestcase() Type: INSERT\n-- Purpose: Validate that inserting a new user_uuid().\n\n-- Enable required extension for gen_random_uuid()\nCREATE EXT-- Goal: Update a user's status to a suspended state and ensure related audit/tracking behavior with an email that duplicates an existing\n-- email (case-insensitive) fails due to a unique constraint onENSION IF NOT EXISTS "pgcrypto";\n\n-- 1) Ensure a canonical role exists (id, name.\n-- Assumptions:\n-- - The user status enum type is named user_status and has a 🔧 saveTestcase() citext email.\n\n-- Assumptions based on schema:\n-- - emails are stored as citext in users.email with a UNIQUE constraint (implied by design)\n-- - Use gen_random_uuid() for)\nWITH existing_role AS (\n SELECT id FROM roles WHERE name = 'user' LIMIT 1\n), created_role AS (\n INSERT INTO roles (id, name, created_at value that can represent 'suspended'.\n-- - We will perform a safe update with optimistic concurrency control via the version column.\n-- - We will update updated_at and updated_by, and optionally 🔧 saveTestcase() UUIDs\n-- - Use appropriate timestamps and default behaviors for created_at/updated_at\n\nBEGIN;\n\n-- Ensure test isolation: clean slate for the users table (optional in real tests; adjust as needed)\n-- DELETE FROM users WHERE email ILIKE '[email protected]';, description)\n SELECT COALESCE((SELECT id FROM existing_role), gen_random_uuid()), 'user', now(), 'Default user role'\n WHERE NOT EXISTS (SELECT 1 FROM roles WHERE name = 'user')\n RETURNING id\n)\nSELECT id FROM created_role\nUNION ALL\nSELECT id FROM existing_role\nINTO TEMP TABLE tmp_role set last_login_at to NULL to invalidate sessions if applicable in application logic.\n-- - We will insert an audit_log entry to record the suspension event.\n\n-- 1) Define the target user by id (replace with actual UUID in real test).\n-- For the purpose of a standalone test, we use a placeholder UUID via gen 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() ⏺ (qa): { -- Insert a sample user status change scenario and generate corresponding audit log entry. --\n\n-- First, insert an initial user to establish an existing case-insensitive duplicate scenario\nINSERT INTO users (\n id,\n email,\n status,\n version,\n _id;\n\n-- 2) Ensure a test user exists to assign the role to\nWITH existing_user AS (\n SELECT id FROM users WHERE email = '[email protected]'::citext LIMIT 1\n), created_user AS (\n INSERT INTO users (\n id, email, status, version, metadata, created_at, created_by, updated_at, updated_by,\n display_name, phone_number, last_login_at, password_hash, email_verified_at\n )\n SELECT\n COALESCE((SELECT id FROM existing_user), gen_random_uuid()),\n '[email protected]'::citext,\n 'active'::user_status,\n 1,\n '{}'::jsonb,\n now(), NULL, now(), NULL,\n 'Test User', NULL, NULL, 'hashed_password_placeholder', NULL\n WHERE NOT EXISTS (SELECT 1 FROM users WHERE email = '[email protected]'::citext)\n RETURNING id\n)\nSELECT id FROM created_user_random_uuid().\n\nDO\n$do$ \nBEGIN\n -- Example: suspend user with a specific id, using a provided current_version for optimistic locking\n -- Replace '00000000-0000-0000-0000-000000000001' with the actual user id in real scenarios.\n -- We also demonstrate handling the case where the user is already suspended or not found by checking rows affected.\n \n -- Parameters ⏺ (qa): { 🔧 saveTestcase() 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() 🔧 saveTestcase()(node:7777) Warning: The 'import-in-the-middle' hook has already been initialized 🔧 saveTestcase()n u.email,\n u.status,\n CASE\n WHEN u.status = 'active' THEN true\n ELSE false\n END AS can_login\nFROM (\n SELECT * FROM users\n) AS u\nWHERE u.status = 'active'\nLIMIT 1;\n\n-- Expected result: A row with can_login = true for an active user; rows for suspended/archived should not appear in this query\n\n" 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() ⏺ (qa): { ⏺ (qa): { ⏺ (qa): { ⏺ (qa): { 🔧 saveTestcase() on users.status with a CHECK constraint\n-- This test case performs an UPDATE that attempts to set an invalid status value,\n-- which should be rejected by the constraint.\n\nBEGIN;\n\n-- 1) Ensure there', 'audit.seed', '{}'::jsonb, NULL, TRUE, NULL, NOW(), 'audit_logs');\n END IF;\nEND\n$$;\n\n-- Validate immutability by applying a policy that prevents updates on immutable records.\n-- This assumes a trigger or policy exists to enforce immutability valid (not expired) token for purpose 'verify_email' or similar.\n-- When the token is consumed (used_at set) and the email remains unchanged, we should:\n-- 1) set email_verified_at on the user,\n-- 2) ensure the user is activated (status not assumes:\n-- ALTER TABLE users ADD CONSTRAINT users_phone_number_unique_unique UNIQUE (phone_number)\n-- BUT only where phone_number is not null is not directly supported in PostgreSQL with a partial unique constraint on existing column value.\n-- We password_resets WHERE used_at IS NULL AND expires_at > now()) THEN\n -- Create a fresh valid reset token for testing\n INSERT INTO password_resets (id, user_id, created_at, expires_at, token_hash, created_by_ip)\n VALUES (gen_random_uuid(), ( is a constraint that restricts status to an allowed set. If not present, add one.\n-- The allowed statuses are assumed to be defined by a CHECK constraint against a known set.\nDO $$\nBEGIN\n IF NOT. If not present, this statement should fail with a permission/trigger exception.\n\nCOMMIT;\n" 🔧 saveTestcase() now(), with AND() OR used_at the intended IS NOT used_at cc.check new phone IS number NULL, raise an NULL\n-- error - value We ( willor when return running perform the_clause 0 I an test.\LIKE id rows '%empot affectedn INent (').\ 🔧 saveTestcase()\nDOn UPDATE-- within $$ Here a\n we implement transaction.%' 'adminBEGIN\n a safe conditional update --n\n Example that will setup updateDO $$ only ( if thecomment\n tokened outDECLARE is valid.\n in' production '%' 🔧 saveTestcase() ) vn\n_user'):UPDATE --_id ensure UUID index := exists ' for uniqueness000 on non-null000 password_res00-000 phone numbers0-\n --0000-ets\n EXECUTE ' placeholder\n 000 ) THEN0-CREATE UNIQUE INDEX000000\n --000001'; IF NOT EXISTS -- target users_phone Create an enum user id_number_unique-like constraint using a for test; replace_idx ON users ( CHECK with aSET used_at = now as needed\nphone_number finite set of v_verification_id UUID) WHERE phone_number IS NOT NULL allowed values.\n ALTER TABLE(), metadata = jsonb_set(coalesce(metadata, '{}' := '11111111-1111-';\n\n -- Example operation: users\n ADD CONSTRAINT users_status_allowed::jsonb), '{attempted}', to_json ⏺ (qa): { 🔧 saveTestcase() label if different\n v_token_hash TEXT := 'REPLACED_BY_TEST_TOKEN_HASH'; -- placeholder for token_hash check\n v_email TEXT;\nBEGIN\n -- 1) Validate token is present, not used, not expired, for this user and purpose 'verify_email'\n TEXT := '+1-555-123-4567';\n -- new_phone TEXT := '+1-555-987-6543';\n -- BEGIN\n\n -- For safety, perform in a transaction so you can easily rollback in test environments\n -- You may remove the following block in updated, the following can be used by tests to assert rejection due to expired/used token.\n-- The test framework should interpret 0 rows returned as a rejection case.\n", 🔧 saveTestcase() verification event\n INSERT INTO audit_logs (notes, action, payload, actor_id, immutable, target_id, created_at, target_table)\n VALUES (\n 'Email verified [email protected]'::citext, 'active', 1, now(), now());\n-- UPDATE users SET phone_number = '+1-555-000-0001' WHERE email = '[email protected]';\n-- UPDATE users a custom enum; cast accordingly\n 1,\n NULL,\n now(),\n NULL,\n NULL,\n now(),\n NULL,\n NULL,\n NULL,\n token: ' || v_verification_id,\n 'email.verification',\n jsonb_build_object('user_id', v_user_id, 'verification_id', v_verification_id, 'verifications SET phone_number = '+1-555-000-0002' WHERE email = '[email protected]';\n-- 2) Attempt to run an update that sets bob's phone to '+1- NULL,\n NULL\n);\n\n-- 2', 'email'),\n v_updated_by,\n TRUE,\n v_user_id::text,\n v_now,\555-000-0001' and observe) Optional:n 'users'\n ); insert unique an audit violation\n ELSE log for\n user .registration if such error.\n-- -- If token was not found policy exists.\n-- If audit_logs has a policy requiring immutable3) Then run a valid update to a new unique number or already used/expired, raise a notice or raise exception as per policy entries, ensure the insert for bob to confirm success.\n R aligns withn\n--AISE Note: NOTICE This ' schema.\ scriptEmail verificationn demonstrates the tokenINSERT enforcement mechanism not INTO. In audit a_logs real found (\ QA or alreadyn run,\ used n notes/-- use,\n explicitexpired UUID:s action and phone %,\n numbers,', v and wrap updates payload_ver in,\ aificationn transaction_id that you actor;\ rollback after verification_idn." ❌ Test Case: Admin invites a user and assigns default roleError:
|
MH4GF
requested review from
MH4GF,
NoritakaIkeda,
Copilot,
junkisai and
sasamuku
and removed request for
a team
There was a problem hiding this comment.
Pull Request Overview
This PR fixes duplicate user messages in DB Agent LLM context by clearing the messages array to prevent unnecessary token consumption. The issue occurred because user messages were being included twice - once in the contextPrompt and once in the messages array from setupWorkflowState.
Key changes:
- Added
messages: []todbAgentStateto eliminate duplicate messages while preserving the prompt field - Follows the same pattern used in the deep modeling flow for consistency
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
1 participant
Issue
Why is this change needed?
User messages were being included twice in the LLM context when using DB Agent, causing unnecessary token consumption:
contextPromptininvokeDesignAgentmessagesarray passed fromsetupWorkflowStateThis fix implements Option 1 from the GitHub issue by setting
messages: []when creatingdbAgentState, following the same pattern used increateGraph.tsfor the deep modeling flow.Changes
messages: []todbAgentStateininvokeDbAgentStream.tsto clear duplicate messages while preserving thepromptfieldReview Focus
SUPABASE_SERVICE_ROLE_KEY, so please test in a proper environmentcreateGraph.tsline 36Link to Devin run: https://app.devin.ai/sessions/e686f1cc21514247bc7a7b9e9d3fcf18
Requested by: @MH4GF