Skip to content

LCORE-1839: E2E for HITL #2115

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 3 commits into
base: main
Choose a base branch
from
Open

LCORE-1839: E2E for HITL #2115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
60d78a9
e2e(hitl): added e2e tests for HITL
jrobertboos Jul 7, 2026
7d6fc22
e2e(hitl): added tests for retention/expiry
jrobertboos Jul 10, 2026
25b7a82
e2e(hitl): skipped tests
jrobertboos Jul 10, 2026
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
34 changes: 34 additions & 0 deletions tests/e2e/configuration/library-mode/lightspeed-stack-mcp-approvals-short-retention.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
name: Lightspeed Core Service (LCS)
service:
host: 0.0.0.0
port: 8080
auth_enabled: false
workers: 1
color_log: true
access_log: true
llama_stack:
# Library mode - embeds llama-stack as library
use_as_library_client: true
library_client_config_path: run.yaml
user_data_collection:
feedback_enabled: true
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
conversation_cache:
type: "sqlite"
sqlite:
db_path: "/tmp/data/conversation-cache.db"
authentication:
module: "noop-with-token"
inference:
default_provider: openai
default_model: gpt-4o-mini
approvals:
approval_timeout_seconds: 60
approval_retention_seconds: 5

@coderabbitai coderabbitai Bot Jul 10, 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.

🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify ApprovalsConfiguration schema for approval_retention_seconds
rg -n -A 30 'class ApprovalsConfiguration' src/models/config.py

# Search for approval_retention_seconds anywhere in the codebase
rg -rn 'approval_retention_seconds' src/

# Check ConfigurationBase extra-field handling
rg -n 'class ConfigurationBase' src/models/config.py
rg -n 'extra|model_config|ConfigDict' src/models/config.py | head -20

Repository: lightspeed-core/lightspeed-stack

Length of output: 1925

approval_retention_seconds is invalid here. src/models/config.py only defines approval_timeout_seconds and approval_retention_days, and ConfigurationBase forbids extra fields. This config will either fail validation at startup or fall back to the 30-day default, so the 6-second purge scenarios won’t work as written. If short-retention e2e coverage is required, add a supported seconds-based retention setting to ApprovalsConfiguration instead of using an unsupported key.

🤖 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/configuration/library-mode/lightspeed-stack-mcp-approvals-short-retention.yaml`
at line 29, The test configuration uses unsupported approval_retention_seconds,
so replace it with a valid configuration field or add a seconds-based retention
option to ApprovalsConfiguration and expose it through ConfigurationBase
validation; update the short-retention purge scenarios to use that supported
setting while preserving their intended timing.

mcp_servers:
- name: "mcp-approval-always"
provider_id: "model-context-protocol"
url: "http://mock-mcp:3000"
require_approval: "always"
34 changes: 34 additions & 0 deletions tests/e2e/configuration/library-mode/lightspeed-stack-mcp-approvals-short-timeout.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
name: Lightspeed Core Service (LCS)
service:
host: 0.0.0.0
port: 8080
auth_enabled: false
workers: 1
color_log: true
access_log: true
llama_stack:
# Library mode - embeds llama-stack as library
use_as_library_client: true
library_client_config_path: run.yaml
user_data_collection:
feedback_enabled: true
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
conversation_cache:
type: "sqlite"
sqlite:
db_path: "/tmp/data/conversation-cache.db"
authentication:
module: "noop-with-token"
inference:
default_provider: openai
default_model: gpt-4o-mini
approvals:
approval_timeout_seconds: 5
approval_retention_seconds: 5
mcp_servers:
- name: "mcp-approval-always"
provider_id: "model-context-protocol"
url: "http://mock-mcp:3000"
require_approval: "always"
46 changes: 46 additions & 0 deletions tests/e2e/configuration/library-mode/lightspeed-stack-mcp-approvals.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: Lightspeed Core Service (LCS)
service:
host: 0.0.0.0
port: 8080
auth_enabled: false
workers: 1
color_log: true
access_log: true
llama_stack:
# Library mode - embeds llama-stack as library
use_as_library_client: true
library_client_config_path: run.yaml
user_data_collection:
feedback_enabled: true
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
conversation_cache:
type: "sqlite"
sqlite:
db_path: "/tmp/data/conversation-cache.db"
authentication:
module: "noop-with-token"
inference:
default_provider: openai
default_model: gpt-4o-mini
approvals:
approval_timeout_seconds: 60
approval_retention_days: 7
mcp_servers:
- name: "mcp-approval-always"
provider_id: "model-context-protocol"
url: "http://mock-mcp:3000"
require_approval: "always"
- name: "mcp-approval-never"
provider_id: "model-context-protocol"
url: "http://mock-mcp:3000"
require_approval: "never"
- name: "mcp-approval-granular"
provider_id: "model-context-protocol"
url: "http://mock-mcp:3000"
require_approval:
always:
- "<PLACEHOLDER>"
never:
- "<PLACEHOLDER>"
Comment on lines +42 to +46

@coderabbitai coderabbitai Bot Jul 10, 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 | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for granular approval scenarios in the feature file
rg -n 'granular' tests/e2e/features/approvals.feature

# Check what tools the mock MCP server exposes
rg -rn 'mock-mcp' tests/e2e/ --type yaml | head -20

Repository: lightspeed-core/lightspeed-stack

Length of output: 3150

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- approvals.feature granular section ---'
sed -n '60,115p' tests/e2e/features/approvals.feature

echo
echo '--- mcp-related config files mentioning tool names or placeholders ---'
rg -n '<PLACEHOLDER>|tool|tools|mcp-approval-granular|mock-mcp' tests/e2e/configuration tests/e2e/features -g '*.yaml' -g '*.feature' -g '*.json' -g '*.ts' -g '*.js' | sed -n '1,220p'

Repository: lightspeed-core/lightspeed-stack

Length of output: 29040

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- server-mode approvals config around granular section ---'
sed -n '30,55p' tests/e2e/configuration/server-mode/lightspeed-stack-mcp-approvals.yaml

echo
echo '--- all occurrences of mcp-approval-granular ---'
rg -n 'mcp-approval-granular' tests/e2e -g '*.yaml' -g '*.feature' -g '*.ts' -g '*.js' | sed -n '1,120p'

echo
echo '--- mock MCP server tool definitions ---'
rg -n 'mock-mcp|tool.*name|name: ".*approval|always|never|granular|approval' tests/e2e -g '*.yaml' -g '*.ts' -g '*.js' | sed -n '1,220p'

Repository: lightspeed-core/lightspeed-stack

Length of output: 9553

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- mock mcp files ---'
fd -a 'mock-mcp|mcp' tests/e2e -t f | sed -n '1,200p'

echo
echo '--- search for tool definitions and approval-granular identifiers ---'
rg -n 'tool|tools|name: "mcp-approval-granular"|list_tools|call_tool|approval' tests/e2e -g '*.ts' -g '*.js' -g '*.yaml' -g '*.json' | sed -n '1,240p'

Repository: lightspeed-core/lightspeed-stack

Length of output: 9816

Replace the granular approval placeholders with real tool identifiers.
tests/e2e/configuration/library-mode/lightspeed-stack-mcp-approvals.yaml:42-46 still uses <PLACEHOLDER> for both always and never, so the granular approval scenarios in tests/e2e/features/approvals.feature won’t exercise the intended approval behavior until these lists point at actual mock-mcp tool names.

🤖 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/configuration/library-mode/lightspeed-stack-mcp-approvals.yaml`
around lines 42 - 46, Replace the <PLACEHOLDER> entries under
require_approval.always and require_approval.never in the MCP approvals
configuration with the actual mock-mcp tool identifiers expected by the granular
scenarios in approvals.feature, assigning each tool to the appropriate approval
list.

@radofuchs radofuchs Jul 22, 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.

agree with coderabbit on this one with one small detail. The point of this PR is to create the feature file. If you intend to create also the config files, than these should be complete, not with these kind of placeholders

35 changes: 35 additions & 0 deletions tests/e2e/configuration/server-mode/lightspeed-stack-mcp-approvals-short-retention.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: Lightspeed Core Service (LCS)
service:
host: 0.0.0.0
port: 8080
auth_enabled: false
workers: 1
color_log: true
access_log: true
llama_stack:
# Server mode - connects to separate llama-stack service
use_as_library_client: false
url: http://${env.E2E_LLAMA_HOSTNAME}:8321
api_key: xyzzy
user_data_collection:
feedback_enabled: true
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
conversation_cache:
type: "sqlite"
sqlite:
db_path: "/tmp/data/conversation-cache.db"
authentication:
module: "noop-with-token"
inference:
default_provider: openai
default_model: gpt-4o-mini
approvals:
approval_timeout_seconds: 60
approval_retention_seconds: 5

@radofuchs radofuchs Jul 22, 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.

this config looks weird, how can this even be allowed to have timeout longer than the retention period?
I would expect this setup to end with an error and be tested in unit test level, rather than here

mcp_servers:
- name: "mcp-approval-always"
provider_id: "model-context-protocol"
url: "http://mock-mcp:3000"
require_approval: "always"
35 changes: 35 additions & 0 deletions tests/e2e/configuration/server-mode/lightspeed-stack-mcp-approvals-short-timeout.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: Lightspeed Core Service (LCS)
service:
host: 0.0.0.0
port: 8080
auth_enabled: false
workers: 1
color_log: true
access_log: true
llama_stack:
# Server mode - connects to separate llama-stack service
use_as_library_client: false
url: http://${env.E2E_LLAMA_HOSTNAME}:8321
api_key: xyzzy
user_data_collection:
feedback_enabled: true
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
conversation_cache:
type: "sqlite"
sqlite:
db_path: "/tmp/data/conversation-cache.db"
authentication:
module: "noop-with-token"
inference:
default_provider: openai
default_model: gpt-4o-mini
approvals:
approval_timeout_seconds: 5
approval_retention_seconds: 5

@radofuchs radofuchs Jul 21, 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.

in case of this config, the retention should not be as low as 5 seconds as it could very easily make the test designed for this pass for incorrect reason

mcp_servers:
- name: "mcp-approval-always"
provider_id: "model-context-protocol"
url: "http://mock-mcp:3000"
require_approval: "always"
47 changes: 47 additions & 0 deletions tests/e2e/configuration/server-mode/lightspeed-stack-mcp-approvals.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
name: Lightspeed Core Service (LCS)
service:
host: 0.0.0.0
port: 8080
auth_enabled: false
workers: 1
color_log: true
access_log: true
llama_stack:
# Server mode - connects to separate llama-stack service
use_as_library_client: false
url: http://${env.E2E_LLAMA_HOSTNAME}:8321
api_key: xyzzy
user_data_collection:
feedback_enabled: true
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
conversation_cache:
type: "sqlite"
sqlite:
db_path: "/tmp/data/conversation-cache.db"
authentication:
module: "noop-with-token"
inference:
default_provider: openai
default_model: gpt-4o-mini
approvals:
approval_timeout_seconds: 60
approval_retention_days: 7
mcp_servers:
- name: "mcp-approval-always"
provider_id: "model-context-protocol"
url: "http://mock-mcp:3000"
require_approval: "always"
- name: "mcp-approval-never"
provider_id: "model-context-protocol"
url: "http://mock-mcp:3000"
require_approval: "never"
- name: "mcp-approval-granular"
provider_id: "model-context-protocol"
url: "http://mock-mcp:3000"
require_approval:
always:
- "<PLACEHOLDER>"
never:
- "<PLACEHOLDER>"
Loading
Loading