Skip to content

LCORE-3027: Debug skills E2E #2138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jrobertboos wants to merge 1 commit into
base: main
Choose a base branch
from
+1 −0
Open

LCORE-3027: Debug skills E2E #2138

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions tests/e2e/features/steps/llm_query_response.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ def wait_for_complete_response(context: Context) -> None:
"""Wait for the response to be complete."""
context.response_data = _parse_streaming_response(context.response.text)
context.response.raise_for_status()
print(f"Response data: {context.response_data}")
Comment on lines 94 to +96

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Emit the diagnostic before raise_for_status().

When the response has a non-2xx status, Line 95 raises before Line 96 executes, so the payload is unavailable for precisely those failures the debug output should help diagnose. Move the safe, redacted diagnostic immediately after parsing or include it in error handling.

🤖 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/e2e/features/steps/llm_query_response.py` around lines 94 - 96, Update
the response handling around _parse_streaming_response so the safe, redacted
diagnostic is emitted immediately after parsing and before
context.response.raise_for_status(). Preserve the existing status validation and
ensure failure responses still print the parsed payload for debugging.

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not print the full response payload to CI logs.

context.response_data includes response text, conversation IDs, tool calls/results, and stream errors. Raw printing can expose sensitive data to anyone with CI-log access and produce unbounded output. Use the project logger with a bounded/redacted diagnostic instead.

As per coding guidelines, module logging must use logger = get_logger(__name__), and sensitive data must not be leaked in logs.

🤖 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/e2e/features/steps/llm_query_response.py` at line 96, Replace the raw
context.response_data print in the response-handling step with the project
logger initialized as logger = get_logger(__name__). Emit only a bounded,
redacted diagnostic that excludes response text, conversation IDs, tool
calls/results, and stream errors; do not log the full payload.

Source: Coding guidelines

assert context.response_data["finished"] is True
context.use_streaming_response_data = True

Expand Down
Loading