✅ Auto-merged (2)
This PR only adds new test cases to two existing test files, validating multi-tenancy isolation in the orchestrator and tenant resolver. No production code is modified, so it cannot break runtime behavior, though it could reveal existing bugs in CI.
- Changes confined entirely to test files (tests/test_orchestrator_temporal.py, tests/test_worker_tenancy.py)
- No modifications to production/business logic, infra, auth, or data layers
- Purely additive (327 lines added, 0 removed) with no deletions or refactors of existing tests
- Worst case impact is a CI failure surfacing a real multi-tenancy bug, not a production incident
Test-only PR adding solid multi-tenancy coverage for QRAD-247 (two tenants through one worker, independent caching, TTL expiry, unconfigured tenant isolation). Tests are well documented, use distinct fixtures per tenant to avoid false positives, and align with existing patterns in the test suite. No correctness, security, or production-code issues found.
No inline comments.
Changes a timestamp formatting helper for Jira enrichment headers (presentation-only, non-idempotency-affecting) and strengthens/expands the LLM system prompt's anti-hallucination rule, with accompanying test coverage. No auth, payments, schema, or infra changes; behavior change is confined to prompt wording and a display string.
- <item>Timestamp change is explicitly presentation-only; docstring confirms it doesn't affect idempotency/marker detection logic</item> <item>Prompt change only tightens instructions to the LLM to avoid hallucinated tech claims, not a functional/business-logic change</item> <item>Well-covered by new and updated tests (test_jira_adapter.py, test_analyse_prompt.py)</item> <item>Small diff confined to two source files plus their tests, no touches to auth, payments, DB schema, or infra</item> </reasons>
Change is low-risk: a presentation-only timestamp formatting helper (well tested) and a prompt-wording strengthening with accompanying tests. No correctness or security issues found; only minor stylistic/readability notes.
🗄️ Closed elsewhere (merged/declined outside this tool) (4)
This is a UI/view-model redesign of the tenant admin approvals page (card layout, pipeline step summarization, new CSS) plus an additive, optional 'created' field on the webhook_bridge PausedWorkflow API response. It does not touch approval decision logic, auth checks, or the actual approve/reject actions, but it is on the admin approvals path and does change a public API response model (backward-compatible) and the internal PausedWorkflow model used by tenant_admin.
- <item>New pure-function module (approval_view.py) is well-tested with extensive unit tests covering token parsing edge cases</item> <item>Changes touch tenant_admin/main.py's approvals_list route, replacing 'paused' context var with 'rows' -- template/context contract change confined to one page</item> <item>webhook_bridge/routes/admin.py adds a new optional field to a public API response model (PausedWorkflow) -- low risk since additive/backward-compatible, but it's a public API contract touched, which raises caution</item> <item>No changes to authentication/authorization, approve/reject decision logic, database schemas, or payment/billing code</item> <item>Diff is fairly large (633 additions) but mostly template/CSS/tests, not sprawling across unrelated systems</item> <item>Affects an admin-facing approval workflow UI; a bug could confuse admins reviewing pending approvals but does not directly enable unauthorized actions</item> </reasons>
Well-structured refactor with good test coverage for the new pure view-model module. Only minor issues found; nothing blocking.
This PR only adds two additional guidance rules (anti-hallucination and markdown formatting) to a system prompt string used in the analyse step, plus new unit tests pinning prompt wording. No control flow, schema, API, or data handling logic is changed.
- Change is confined to a prompt string in core/orchestrator.py plus a new test file", "No changes to business logic, schemas, APIs, auth, or infra", "Small diff size (94 lines added, 0 removed)", "Worst-case impact is degraded LLM prompt behavior, not a functional break", "Well covered by new regression tests for the prompt content"]}
Small, low-risk prompt-text addition plus dedicated regression tests. The new rules read clearly and the tests are well scoped to string-builder functions with no external dependencies. No correctness or security issues found; one minor suggestion below.
This PR adds only new, explicitly local-only test/dev-infrastructure files (embedding provider stub, local activities substitute, worker wiring, and several E2E/smoke test files) plus corresponding tests. No production files (worker.py, rag_indexing/*, connectors/*, llm_provider.py) are modified, and the new files are documented as not tracked/deployed and guarded against accidental use against real Key Vault/production DSNs.
- All changed/added files are net-new (1713 additions, 0 deletions) and confined to local-dev/testing infrastructure, not production code paths.
- Extensive docstrings and guard logic (vault URL checks, DSN checks, tenant checks) explicitly prevent this code from running against real Azure Key Vault, production DB, or production tenants.
- No modification to authentication, billing, database schema/migrations, or production API contracts.
- New tests include explicit checks (hash-based) verifying that key production files (activities.py, worker.py, workflow.py, db.py, bitbucket_skills.py, worker_tenancy.py) remain unmodified.
- Credentials/secrets handling is read from local .env.local only, never logged, and isolated from the shared os.environ used by other tests.
- Even though the diff is sizeable (1713 lines), it is additive test/dev tooling rather than sprawling changes across unrelated production areas.
This PR adds a large set of local-only, git-untracked developer tooling and tests (Ollama embedding provider shim, local RAG activities substitute, and several live/manual smoke tests) for QRAD-288. The code is heavily documented, guarded against accidental production use (vault-URL checks, tenant checks, presence-only logging), and the tests look thorough. Only a couple of minor, non-blocking issues found; nothing that should block merge given this is explicitly local-only, non-production, non-deployed tooling.
This PR fixes a security-relevant bug in the orchestrator where dangerous_instructions guardrail findings were computed but silently discarded, causing potentially harmful tickets to bypass human approval. It changes core authorization/guardrail merge logic that gates whether human review is required, affecting the safety-critical approval pipeline for all ticket processing.
- Touches core/orchestrator.py's guardrail evaluation and resolve_execution_authorization call path, which determines whether human approval is bypassed
- Changes security-sensitive fail-closed behavior (ADR-6 part 5) around a classifier used to detect dangerous/harmful instructions
- Previously silent finding-suppression bug being fixed here means behavior change could newly trigger (or fail to trigger) human approval gates across all tenants
- Modifies shared data models (ContentGuardrailFinding -> GuardrailFinding) and merges findings into a unified guardrail_result used by multiple call sites
- Mistakes here could either cause false negatives (dangerous content slipping through) or over-blocking (excessive human approval gates), both high impact for a safety/security control
This is a well-reasoned bugfix: it properly merges the dangerous_instructions classifier findings into the same guardrail_result instead of discarding them, and replaces the "None guardrail_result" fail-closed hack with an explicit failed_checks entry that is now surfaced in checks_run/authorization_reason. Tests are updated accordingly and add a new regression test for the previously-silent finding-suppression bug. No correctness, security, or edge-case issues found in the diff.
No inline comments.