Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 56 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Agent Guidance for O-QT MCP Server

## Scope
This file applies to the `o-qt-mcp-server-public` repository (canonical target).

## Critical Controls (Added 2026-04-16)

### 1. Applicability Domain (AD) Gating
- **Files:** `src/tools/implementations/o_qt_qsar_tools.py`, `src/tools/implementations/toolbox_execution.py`, `src/tools/implementations/workflow_runner.py`
- **Behavior:** `run_qsar_prediction` and `run_qsar_model` now inspect the domain result from the QSAR Toolbox. If the domain status is `"OutOfDomain"`, the result includes:
- `"ad_status": "out_of_domain"`
- `"ad_warning": true`
- `"ad_recommendation": "..."`
- **Rule:** Do NOT remove these fields. The workflow runner surfaces AD warnings in the Markdown summary.

### 2. Human Review Checkpoints (OQT-02)
- **Files:** `src/utils/review.py`, `src/tools/implementations/workflow_runner.py`, `config/tool_permissions.default.json`
- **Behavior:** When `require_human_review=true` is passed to `run_oqt_multiagent_workflow`, the workflow creates up to three checkpoints:
1. `chemical_identity` — after resolving the input identifier to a Toolbox record
2. `ad_assessment` — when any QSAR prediction reports `ad_warning=true`
3. `final_report` — before generating the PDF artifact
- If checkpoints are pending, the workflow returns `status: "review_required"` with `workflow_id` and `review_checkpoints`. No PDF is generated.
- Clients can approve/reject checkpoints via the `approve_workflow_checkpoint` tool, then resume by passing the same `workflow_id` (and optionally `checkpoint_approvals`) to the workflow.
- **Rule:** Do NOT auto-generate artifacts when `require_human_review=true` and checkpoints are pending. Do NOT skip the `ad_assessment` checkpoint for out-of-domain predictions.

### 3. LLM Prompt-Boundary Sanitization
- **File:** `src/utils/sanitization.py`, `src/integrations/oqt_assistant.py`
- **Behavior:** All user-supplied identifiers and context strings are sanitized with `sanitize_for_llm()` before entering the oqt_assistant LLM pipeline.
- **Rule:** If you add new LLM-facing inputs, pipe them through `sanitize_for_llm()`.

### 4. Privacy-Aware Audit Logging
- **Files:** `src/utils/privacy.py`, `src/tools/registry.py`, `src/api/server.py`, `src/utils/logging.py`
- **Behavior:**
- Audit events hash SMILES, CAS numbers, chemical names, and API keys before logging.
- The HTTP audit middleware parses query strings into dictionaries so parameter keys remain readable while values are hashed.
- The `PrivacyLogFilter` scrubs SMILES/CAS patterns from free-text log messages and URL query parameters, and hashes whole-value identifiers in structured log extra fields.
- **Rule:** Do NOT log raw chemical identifiers or secrets. Use `scrub_dict()` on params before audit emit.

### 5. Fallback PDF Provenance
- **File:** `src/utils/pdf_generator.py`
- **Behavior:** The fallback PDF includes:
- A prominent disclaimer on the first page
- An "Applicability Domain Warnings" section when out-of-domain predictions are present
- A "Provenance" section showing model count and AD status
- **Rule:** Keep the disclaimer visible. Do not remove the AD-warning block.

### 6. Search Defaults
- **File:** `src/tools/implementations/workflow_runner.py`, `src/tools/implementations/o_qt_qsar_tools.py`
- **Behavior:** `search_type` default is now `"name"` instead of `"auto"` to reduce silent wrong-chemical resolution.
- **Rule:** Do not revert the default to `"auto"` without explicit user confirmation logic.

## Testing Expectations
- Any change to AD logic must pass `test_run_qsar_prediction_ad_warning_out_of_domain`.
- Any change to privacy logic must pass `tests/utils/test_privacy.py`.
- Any change to sanitization must pass `tests/utils/test_sanitization.py`.
- Any change to PDF generation must pass `test_generate_pdf_report_includes_disclaimer_and_ad_warnings`.
25 changes: 24 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

---

## [0.3.1] - 2026-04-17

### Added
- **Human review checkpoints** (`OQT-02`). When `require_human_review=true` is passed to `run_oqt_multiagent_workflow`, the workflow now pauses at up to three checkpoints (`chemical_identity`, `ad_assessment`, `final_report`) and returns `status: "review_required"` instead of auto-generating artifacts.
- `approve_workflow_checkpoint` tool to explicitly approve or reject pending workflow checkpoints.
- **Applicability-domain hard gating** (`OQT-01`). Out-of-domain predictions block PDF generation when `require_human_review=true`.
- `PrivacyLogFilter` (`OQT-05`) to scrub SMILES, CAS numbers, and chemical names from free-text log messages and URL query parameters before emission.
- `sanitize_for_llm()` utility (`OQT-04`) to strip control characters, backticks, and dollar signs from untrusted identifiers before they enter LLM-facing contexts.
- Fallback PDF provenance enhancements (`OQT-03`): disclaimer header, applicability-domain warnings section, and provenance summary showing models run and AD warning count.
- `qsar_models_executed` field in the workflow response for full QSAR transparency (`MD-004`).
- Safer search defaults (`HG-001`): `search_type` now defaults to `"name"` instead of `"auto"`.
- Unit tests for sanitization, privacy scrubbing, and review checkpoint orchestration.

### Changed
- HTTP audit middleware now parses query strings into dictionaries so parameter keys remain readable while values are hashed.
- Updated `AGENTS.md` with critical-control policies to prevent future regressions of AD gating, sanitization, privacy, and review defaults.

### Fixed
- Removed raw chemical identifiers from audit logs; all SMILES, CAS, and chemical-name values are now hashed before emission.

---

## [0.3.0] - 2026-04-08

### Added
Expand Down Expand Up @@ -60,7 +82,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Dockerfile and docker-compose stack for local development with Toolbox API stub.
- Taskmaster backlog, documentation set (`docs/auth_testing.md`, `docs/observability.md`, `docs/testing.md`), and CI workflow skeleton.

[Unreleased]: https://github.com/ToxMCP/oqt-mcp/compare/v0.3.0...HEAD
[Unreleased]: https://github.com/ToxMCP/oqt-mcp/compare/v0.3.1...HEAD
[0.3.1]: https://github.com/ToxMCP/oqt-mcp/releases/tag/v0.3.1
[0.3.0]: https://github.com/ToxMCP/oqt-mcp/releases/tag/v0.3.0
[0.2.0]: https://github.com/ToxMCP/oqt-mcp/releases/tag/v0.2.0
[0.1.0]: https://github.com/ToxMCP/oqt-mcp/releases/tag/v0.1.0
44 changes: 44 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,9 @@ See [docs/auth_testing.md](docs/auth_testing.md) for token generation tips and b
| `llm_provider` | string | – | Override LLM provider (e.g., `openai`, `openrouter`). |
| `llm_model` | string | – | LLM model identifier. |
| `llm_api_key` | string | – | API key when not provided via environment. |
| `require_human_review` | boolean | – | When `true`, high-risk checkpoints require explicit approval before artifacts are generated. |
| `workflow_id` | string | – | Optional workflow ID for resuming a review-paused workflow. |
| `checkpoint_approvals` | array[{checkpoint_id, decision, comments}] | – | Pre-approved checkpoints to resume a paused workflow. |

### `build_grouping_justification` parameters

Expand Down Expand Up @@ -526,12 +529,53 @@ Portable cross-suite contract files are versioned separately under `schemas/`.

---

## Human review checkpoints

For scientific governance, `run_oqt_multiagent_workflow` supports an optional `require_human_review=true` mode that pauses the workflow at high-risk decision points instead of auto-generating artifacts.

### Checkpoints created
1. **`chemical_identity`** — After resolving the input identifier to a Toolbox record.
2. **`ad_assessment`** — When any QSAR prediction reports `ad_warning=true` (out of applicability domain).
3. **`final_report`** — Before generating the PDF artifact.

### Workflow behavior
- If no checkpoints are triggered, the workflow completes normally (`status: "ok"`).
- If checkpoints are pending, the workflow returns:
- `status: "review_required"`
- `workflow_id` — ID to use when resuming
- `review_checkpoints` — List of pending checkpoints with metadata
- **No PDF is generated.**

### Approving or rejecting checkpoints
Use the `approve_workflow_checkpoint` tool:

```json
{
"name": "approve_workflow_checkpoint",
"arguments": {
"checkpoint_id": "<checkpoint-id>",
"decision": "approved",
"comments": "Looks correct"
}
}
```

Decisions: `approved` | `rejected`

### Resuming the workflow
Pass the same `workflow_id` back to `run_oqt_multiagent_workflow` (with `require_human_review=true`). The server will detect approved checkpoints and complete the workflow, returning `status: "ok"` and the PDF.

> **Note:** Checkpoint state is held in memory. If the server restarts, pending checkpoints are lost. Do not use this feature for long-lived review cycles without external persistence.

---

## Security checklist

- ✅ Use OAuth2/OIDC in production (`BYPASS_AUTH=false`).
- ✅ Terminate TLS at a reverse proxy.
- ✅ Configure RBAC in `config/tool_permissions.default.json`.
- ✅ Enable audit log shipping (see [docs/observability.md](docs/observability.md)).
- ✅ Turn on `require_human_review=true` for high-stakes workflows to enforce explicit checkpoint approval.
- ✅ Rotate secrets via platform-specific secret stores.
- ✅ Regularly update the Docker base image (see [Dockerfile](Dockerfile)).

Expand Down
4 changes: 4 additions & 0 deletions config/tool_permissions.default.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"list_qsar_models",
"list_all_qsar_models",
"build_grouping_justification",
"approve_workflow_checkpoint",
"run_oqt_multiagent_workflow",
"run_qsar_workflow",
"canonicalize_structure",
Expand All @@ -38,6 +39,7 @@
"list_qsar_models",
"list_all_qsar_models",
"build_grouping_justification",
"approve_workflow_checkpoint",
"run_oqt_multiagent_workflow",
"run_qsar_workflow",
"run_qsar_model",
Expand Down Expand Up @@ -72,6 +74,7 @@
"list_qsar_models",
"list_all_qsar_models",
"build_grouping_justification",
"approve_workflow_checkpoint",
"run_oqt_multiagent_workflow",
"run_qsar_workflow",
"run_qsar_model",
Expand Down Expand Up @@ -106,6 +109,7 @@
"list_qsar_models",
"list_all_qsar_models",
"build_grouping_justification",
"approve_workflow_checkpoint",
"run_oqt_multiagent_workflow",
"run_qsar_workflow",
"run_qsar_model",
Expand Down
19 changes: 17 additions & 2 deletions src/api/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,29 @@ async def add_security_headers(request: Request, call_next):
# Placeholder: In production, this must log to a centralized, tamper-evident system.
@app.middleware("http")
async def audit_log_middleware(request: Request, call_next):
from urllib.parse import parse_qs
from src.utils.privacy import scrub_dict, scrub_value

correlation_id = str(uuid.uuid4())
request.state.correlation_id = correlation_id
start = time.perf_counter()

# Scrub potentially sensitive path segments (e.g., SMILES in URL paths)
scrubbed_path = scrub_value("path", request.url.path)
query_str = str(request.url.query) if request.url.query else ""
# Parse query string into a dict so keys remain readable and only values are scrubbed
if query_str:
parsed_query = parse_qs(query_str, keep_blank_values=True)
scrubbed_query = scrub_dict(parsed_query)
else:
scrubbed_query = {}

log.debug(
"Incoming request",
extra={
"cid": correlation_id,
"method": request.method,
"path": request.url.path,
"path": scrubbed_path,
},
)

Expand All @@ -120,10 +133,12 @@ async def audit_log_middleware(request: Request, call_next):
"correlation_id": correlation_id,
"user_id": user_id,
"method": request.method,
"path": request.url.path,
"path": scrubbed_path,
"status_code": response.status_code,
"duration_ms": round(duration_ms, 3),
}
if scrubbed_query:
event["query"] = scrubbed_query
audit.emit(event)

response.headers["X-Request-ID"] = correlation_id
Expand Down
18 changes: 13 additions & 5 deletions src/integrations/oqt_assistant.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
from dataclasses import dataclass
from typing import Any, Dict, List, Optional, Tuple

from src.utils.sanitization import sanitize_for_llm

logger = logging.getLogger(__name__)

try:
Expand Down Expand Up @@ -313,21 +315,26 @@ async def _run_agents(
"""
Execute the oqt_assistant specialist agents and synthesiser.
"""
# Harden LLM-facing inputs against prompt-boundary confusion (OQT-04 / SEC-02)
safe_identifier = sanitize_for_llm(identifier)
safe_context = sanitize_for_llm(context)

specialist_outputs: Dict[str, str] = {}
logger.info(" [Analysis] Starting assistant agents.")

try:
identity_txt = await analyze_chemical_context(
{"basic_info": bundle.get("chemical_data", {}).get("basic_info", {})},
context,
safe_context,
llm_config,
)
except Exception as exc: # pragma: no cover - defensive branch
logger.error("Chemical context agent failed: %s", exc)
identity_txt = f"[Chemical Context agent failed: {exc}]"

specialist_outputs["Chemical_Context"] = identity_txt
analysis_context = f"{identity_txt}\n\nUser Goal: {context}"
safe_identity_txt = sanitize_for_llm(identity_txt)
specialist_outputs["Chemical_Context"] = safe_identity_txt
analysis_context = f"{safe_identity_txt}\n\nUser Goal: {safe_context}"

props = bundle.get("chemical_data", {}).get("properties", {})
profiling = bundle.get("profiling", {})
Expand Down Expand Up @@ -389,11 +396,12 @@ async def _run_agents(
logger.error("Read-Across agent failed: %s", exc)
read_across = f"[Read Across agent failed: {exc}]"

specialist_outputs["Read_Across"] = read_across
safe_read_across = sanitize_for_llm(read_across)
specialist_outputs["Read_Across"] = safe_read_across

try:
final_report = await synthesize_report(
identifier, core_outputs, read_across, context, llm_config
safe_identifier, core_outputs, safe_read_across, safe_context, llm_config
)
except Exception as exc: # pragma: no cover - defensive branch
logger.error("Report synthesis failed: %s", exc)
Expand Down
2 changes: 2 additions & 0 deletions src/qsar/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ async def _execute_request() -> Tuple[Any, Dict[str, Any]]:
"last_attempt_ms": round(elapsed_attempt, 3),
"timeout_profile": profile,
"status_code": response.status_code,
"api_versions": response.headers.get("api-supported-versions"),
"server_date": response.headers.get("date"),
}
return data, meta

Expand Down
19 changes: 17 additions & 2 deletions src/tools/implementations/o_qt_qsar_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class ChemicalSearchParams(BaseModel):
..., description="The search term (Name, CAS number, or SMILES)."
)
search_type: str = Field(
"auto", description="Type of search (e.g., 'auto', 'name', 'cas', 'smiles')."
"name", description="Type of search (e.g., 'auto', 'name', 'cas', 'smiles')."
)


Expand Down Expand Up @@ -375,13 +375,28 @@ async def run_qsar_prediction(smiles: str, model_id: str) -> dict:
)
return _attach_toolbox(result, toolbox_meta)

# Light-weight applicability-domain gating (OQT-01)
domain_value = ""
if isinstance(domain, dict):
domain_value = domain.get("DomainResult") or domain.get("Domain") or ""
elif isinstance(domain, str):
domain_value = domain
domain_normalized = str(domain_value).strip().replace(" ", "").replace("-", "").lower()
ad_warning = domain_normalized in {"outofdomain", "out_of_domain"}

result = {
"chem_id": chem_id,
"model_id": model_id,
"prediction": prediction,
"domain": domain,
"search_hits": hits,
"ad_status": "out_of_domain" if ad_warning else ("in_domain" if domain_normalized in {"indomain", "in_domain", "insideapplicabilitydomain"} else "unknown"),
"ad_warning": ad_warning,
}
if ad_warning:
result["ad_recommendation"] = (
"This prediction is outside the model's applicability domain. "
"Treat with caution and consider experimental validation or read-across."
)
if model_provenance:
result["model_provenance"] = model_provenance
toolbox_meta = _aggregate_meta(
Expand Down
22 changes: 22 additions & 0 deletions src/tools/implementations/toolbox_execution.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,12 +254,34 @@ async def run_qsar_model(qsar_guid: str, chem_id: str) -> dict:
_format_meta("qsar/domain", domain_meta),
_format_meta("about/object", model_meta),
)
# Light-weight applicability-domain gating (OQT-01)
domain_value = ""
if isinstance(domain, dict):
domain_value = domain.get("DomainResult") or domain.get("Domain") or ""
elif isinstance(domain, str):
domain_value = domain
domain_normalized = str(domain_value).strip().replace(" ", "").replace("-", "").lower()
ad_warning = domain_normalized in {"outofdomain", "out_of_domain"}

result = {
"qsar_guid": qsar_guid,
"chem_id": chem_id,
"prediction": prediction,
"domain": domain,
"ad_status": "out_of_domain"
if ad_warning
else (
"in_domain"
if domain_normalized in {"indomain", "in_domain", "insideapplicabilitydomain"}
else "unknown"
),
"ad_warning": ad_warning,
}
if ad_warning:
result["ad_recommendation"] = (
"This prediction is outside the model's applicability domain. "
"Treat with caution and consider experimental validation or read-across."
)
if model_provenance:
result["model_provenance"] = model_provenance
return _attach_toolbox(result, toolbox_meta)
Expand Down
Loading
Loading