dev->main

[whiterabbit1983]

PR Type

Bug fix, Enhancement, Tests

---

Description

• Improved error handling for Jinja2 template rendering:

  • Catch TemplateSyntaxError and UndefinedError to return HTTP 400 errors.
  • Added fallback for unexpected exceptions with HTTP 500 errors.

• Enhanced database query logic:

  • Added stricter filtering and validation for session and execution queries.
  • Introduced search_window for time-based filtering in queries.

• Updated database schema:

  • Adjusted compression policies for entries and transitions tables.
  • Improved error handling during schema changes.

• Added test improvements for execution queries.

---

Changes walkthrough 📝

	Relevant files
Enhancement	10 files pg.py Support multiple JSON types for database connections          +7/-6      get_history.py Refine session history query with stricter filters              +20/-15  list_entries.py Add time-based filtering to list entries query                      +11/-4    count_executions.py Add time-based filtering for execution count query              +1/-0      get_paused_execution_token.py Add time-based filtering for paused execution token query +6/-1      list_execution_transitions.py Add time-based filtering for execution transitions query  +7/-2      list_executions.py Improve sorting logic for execution listing                            +8/-9      prepare_execution_input.py Simplify query structure for execution input preparation  +9/-12    000034_switch_to_hypercore.down.sql Revert hypercore changes with improved error handling        +12/-48  000034_switch_to_hypercore.up.sql Apply hypercore compression policies with error handling  +54/-58
Bug fix	1 files template.py Add error handling for Jinja2 template rendering                  +28/-10
Tests	1 files test_execution_queries.py Add test for execution listing with updated sorting            +2/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-for-open-source]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Missing Null Check The _transform function assumes row["entries"] and row["relations"] are not None when accessing them, but only checks for None when iterating. This could cause errors if the entire fields are None. def _transform(row): return { "entries": [ { **entry, } for entry in (row["entries"] or []) ], "relations": [ { "head": r["head"], "relation": r["relation"], "tail": r["tail"], } for r in (row["relations"] or []) ], "session_id": row["session_id"], "created_at": row["created_at"] or utcnow(), } SQL Query Mismatch The SQL query has been updated to support sorting by both created_at and updated_at, but the parameters passed to the query don't match the placeholders in the query string. WHERE developer_id = $1 AND task_id = $2 ORDER BY CASE WHEN $3 = 'created_at' AND $4 = 'asc' THEN created_at END ASC NULLS LAST, CASE WHEN $3 = 'created_at' AND $4 = 'desc' THEN created_at END DESC NULLS LAST, CASE WHEN $3 = 'updated_at' AND $4 = 'asc' THEN updated_at END ASC NULLS LAST, CASE WHEN $3 = 'updated_at' AND $4 = 'desc' THEN updated_at END DESC NULLS LAST LIMIT $5 OFFSET $6; Exception Handling The template rendering function now catches all exceptions and returns HTTP 500, which might mask programming errors that should be properly logged rather than caught and converted to HTTP responses. except Exception as e: raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=f"Template rendering error: {e!s}", )

[qodo-merge-for-open-source]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Add execution ownership verification The query doesn't verify that the user requesting the paused execution token is the owner of the execution. This could allow unauthorized access to execution tokens. Add a check to verify the developer_id matches the execution owner. agents-api/agents_api/queries/executions/get_paused_execution_token.py [24-29] -FROM latest_transitions +FROM latest_transitions lt +JOIN executions e ON lt.execution_id = e.execution_id WHERE - execution_id = $1 - AND created_at >= $2 - AND created_at >= (select created_at from executions where execution_id = $1) - AND type = 'wait'; + lt.execution_id = $1 + AND lt.created_at >= $2 + AND lt.created_at >= e.created_at + AND e.developer_id = $3 + AND lt.type = 'wait'; Apply this suggestion Suggestion importance[1-10]: 9 __ Why: This suggestion fixes a critical security vulnerability by adding proper ownership verification. Without this check, users could potentially access execution tokens belonging to other developers, which is a serious authorization flaw.	High
	Improve session ownership verification The query joins sessions and filters on both s.session_id = $1 and s.developer_id = $3, but this creates a potential security issue. If a developer tries to access a session they don't own, the query will return no results but won't explicitly check ownership. Add a separate check to verify the developer owns the session. agents-api/agents_api/queries/entries/get_history.py [12-33] -WITH collected_entries AS ( +WITH session_check AS ( + SELECT session_id, created_at AS session_created_at + FROM sessions + WHERE session_id = $1 AND developer_id = $3 +), +collected_entries AS ( SELECT e.entry_id AS id, e.session_id, e.role, e.name, e.content, e.source, e.token_count, e.created_at, e.timestamp, e.tool_calls, e.tool_call_id, e.tokenizer, - s.created_at AS session_created_at + sc.session_created_at FROM entries e - JOIN sessions s ON s.session_id = e.session_id - AND s.session_id = $1 - AND s.developer_id = $3 + JOIN session_check sc ON sc.session_id = e.session_id WHERE e.source = ANY($2) - AND e.created_at >= s.created_at + AND e.created_at >= sc.session_created_at ), Apply this suggestion Suggestion importance[1-10]: 8 __ Why: This suggestion improves security by separating the session ownership check into a dedicated CTE, making the authorization model more explicit and robust. It prevents potential security issues where a developer might try to access sessions they don't own.	Medium
	Prevent error information leakage The generic exception handler exposes internal error details to the client, which could potentially leak sensitive information. It's better to log the full exception details internally but return a more generic error message to the client. agents-api/agents_api/common/utils/template.py [60-64] except Exception as e: + # Log the full exception details for debugging + import logging + logging.error(f"Template rendering error: {e!s}", exc_info=True) raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=f"Template rendering error: {e!s}", + detail="An internal error occurred during template rendering", ) Apply this suggestion Suggestion importance[1-10]: 7 __ Why: The suggestion addresses a security concern by preventing detailed error information from being exposed to clients. Adding proper logging while returning a generic error message is a security best practice that prevents potential information leakage.	Medium
More

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 8eb0612 in 1 minute and 34 seconds

More details

• Looked at 648 lines of code in 12 files
• Skipped 0 files when reviewing.
• Skipped posting 16 drafted comments based on config settings.

1. agents-api/agents_api/clients/pg.py:9

• Draft comment:
  Looping over ['json', 'jsonb'] is clear. Consider adding a comment why both types are configured, in case future types need support.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking for an explanation to be added, which is not allowed by the rules. It doesn't provide a specific code suggestion or point out a potential issue with the code itself. Therefore, it should be removed.

2. agents-api/agents_api/common/utils/template.py:39

• Draft comment:
  Good try/except handling for template rendering. Ensure error messages do not leak sensitive info.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

3. agents-api/agents_api/queries/entries/get_history.py:12

• Draft comment:
  The query now selects 'session_created_at' to return as created_at. Verify that aggregation with potential multiple rows returns a consistent single value.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the author to verify the behavior of the query, which is against the rules. It doesn't provide a specific suggestion or point out a clear issue with the code. It is more of a request for the author to double-check their work, which is not allowed.

4. agents-api/agents_api/queries/entries/list_entries.py:44

• Draft comment:
  Additional created_at constraints look good; ensure that comparing entry created_at with session created_at works as intended and is well-indexed.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the PR author to ensure that a comparison works as intended, which violates the rule against asking for confirmation or ensuring behavior. It does not provide a specific suggestion or point out a clear issue.

5. agents-api/agents_api/queries/executions/count_executions.py:13

• Draft comment:
  The condition on created_at (comparing with developers.created_at) might help filter recent executions; consider performance implications if table grows large.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  The comment is purely informative/speculative about potential performance implications. It doesn't identify a specific problem or suggest a concrete change. It uses language like "might help" and "consider" which indicates uncertainty. Performance concerns would need more context/evidence to be actionable.
  Maybe there really is a performance issue here that needs to be addressed? The subquery could potentially be inefficient.
  Without specific evidence of a performance problem or a concrete suggestion for improvement, this is just speculation. The database will likely optimize this simple subquery well.
  Delete this comment as it's purely informative/speculative and doesn't require any specific code changes.

6. agents-api/agents_api/queries/executions/get_paused_execution_token.py:51

• Draft comment:
  Execution ID is converted to string. Ensure consistency with other queries expecting UUIDs or strings.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. agents-api/agents_api/queries/executions/list_execution_transitions.py:19

• Draft comment:
  The placeholder for scope_id changes from $6 to $7. Verify that parameter ordering in the params array remains correct, especially when scope_id is omitted.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 30% <= threshold 50%
  The comment is asking the author to verify the parameter ordering, which is against the rules as it asks for confirmation or verification. However, it does point out a specific change in the code that could lead to an issue if not handled correctly. The comment could be rephrased to suggest checking the parameter ordering without asking for verification.

8. agents-api/agents_api/queries/executions/list_executions.py:38

• Draft comment:
  Updated sorting conditions now include 'updated_at'. Confirm that interpolation of sort fields is safe and test cases cover both created_at and updated_at sorting.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 30% <= threshold 50%
  The comment is asking the PR author to confirm the safety of interpolation and to ensure test cases cover specific scenarios. This violates the rule against asking the author to confirm or ensure things. However, the comment could be useful if it specifically suggested adding test cases for created_at and updated_at sorting.

9. memory-store/migrations/000034_switch_to_hypercore.up.sql:3

• Draft comment:
  Migration script now uses compression policies instead of hypercore access method. Verify that these changes are idempotent and thoroughly tested in production-like environments.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the PR author to verify that the changes are idempotent and thoroughly tested, which violates the rule against asking for confirmation or testing. It does not provide a specific code suggestion or point out a specific issue with the code.

10. agents-api/agents_api/clients/pg.py:9

• Draft comment:
  Looping over ['json', 'jsonb'] codecs is clear. Confirm that using json.dumps/loads is appropriate for both types in your schema.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

11. agents-api/agents_api/common/utils/template.py:50

• Draft comment:
  Exposing raw exception messages in HTTP responses (e.g. TemplateSyntaxError and UndefinedError) may leak internal details; consider sanitizing these in production.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

12. agents-api/agents_api/queries/entries/get_history.py:67

• Draft comment:
  Using utcnow() as a fallback for 'created_at' may mask missing data; please confirm that substituting the current time is the intended behavior.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  The comment asks the author to "confirm" the behavior, which violates our rules about not asking for confirmations. The code change appears intentional - they're getting created_at from the session table and using utcnow() as a fallback, which is a common pattern. The SQL query's structure suggests this is well thought out.
  Maybe there could be a legitimate data integrity concern here that deserves attention? The fallback could hide bugs.
  While data integrity is important, the comment merely asks for confirmation rather than identifying a specific issue. The code appears to handle the edge case intentionally and safely.
  Delete this comment as it violates our rule about not asking authors to confirm their intentions, and doesn't identify a clear problem that needs fixing.

13. agents-api/agents_api/queries/entries/list_entries.py:44

• Draft comment:
  There are two created_at conditions ('e.created_at >= $7' and 'e.created_at >= (select created_at from sessions where session_id = $1)'). Verify that both filters are intended and do not conflict.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  The two conditions appear to be intentionally added together - one ensures we only look at entries within the search window, and the other ensures we don't look at entries created before the session itself. This is logical and there's no apparent conflict. The comment is asking for verification without identifying any actual problem. This violates the rule about not asking authors to verify their intentions.
  Perhaps there could be a legitimate performance concern about having two timestamp conditions that could be combined. Maybe I'm missing some edge case where these conditions could conflict.
  The conditions serve distinct purposes and are clearly intentional. If there were performance issues, they would be caught in testing. Without strong evidence of a problem, we shouldn't keep speculative comments.
  Delete this comment as it merely asks for verification without identifying any actual issue, violating our rules about not asking authors to confirm their intentions.

14. agents-api/agents_api/queries/executions/list_execution_transitions.py:19

• Draft comment:
  The parameter index for scope_id has shifted (using $7) with the added search_window filter. Ensure that the parameter ordering is maintained consistently.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

15. agents-api/agents_api/queries/executions/list_executions.py:39

• Draft comment:
  The ORDER BY clause now supports sorting by 'updated_at'. Ensure that there are appropriate indexes on 'updated_at' for optimal performance.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

16. memory-store/migrations/000034_switch_to_hypercore.up.sql:6

• Draft comment:
  Migration script updates the compression settings for 'entries' and 'transitions'. Verify that these new TimescaleDB compression policies match your performance and data retention requirements.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

Workflow ID: wflow_S5M9XoFoHtcirmyN

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.