fix(chat-knowledge): contexts can be deleted, cascade with their chat, and orphans get cleaned (#16490)

[mrveiss]

Closes #16490

Single-issue rationale: #16490 covers deleting chat-knowledge contexts: an authorized DELETE route, the cascade from chat and session delete, and orphan discovery and cleanup. No other open issue shares that scope. #16502, which reports the cascade result in the session-delete response, was split out because schemas_chat.py sits exactly at its file-size ceiling and needs a module split first. #15160, the never-assigned chat_knowledge_manager module global, is a different defect with its own fix.

Thinking Path

A test probe on 2026-09-12 left an empty chat-knowledge context on an install, and there was no supported way to remove it. Contexts could be created and read but never deleted, and chat deletion didn't reach them. The fix needed an authorized delete, a cascade from the existing session-delete path, and a way to find contexts whose chat no longer exists.

All chat-knowledge state is in memory on ChatKnowledgeManager (chat_contexts, file_associations, pending_decisions), so there are no DB or Redis rows to migrate. Persistent KB facts stay with _cleanup_knowledge_base_facts, as before.

What Changed

• api/chat_knowledge_delete.py (new, mounted from chat_knowledge.py, the way chat.py mounts chat_sessions.py):
  • DELETE /api/chat-knowledge/context/{chat_id} removes the context, its temporary knowledge, its file associations, and files that upload_file_to_chat itself wrote. Files that are only associated are left alone.
  • Existence is checked with peek_chat_knowledge_manager before authorization, so an unknown id is a 404 and is never claimed by validate_chat_ownership's legacy-migration path.
  • Authorization reuses validate_chat_ownership: owner, org admin, or feat(knowledge): Share KB facts when sharing conversations #689 shared access.
• Orphans: admin-only GET / DELETE /api/chat-knowledge/context-orphans, dry-run by default, the same shape as /session-orphans. The path is hyphenated so /context/{chat_id} can never shadow it.
• Cascade: _perform_all_session_cleanup runs for both DELETE /chats/{chat_id} and DELETE /chat/sessions/{session_id}, and now also calls _cleanup_chat_knowledge_context.
• api/chat_sessions_delete_cleanup.py (new): the six existing KB-fact and transcript cleanup helpers move here verbatim, which makes room for the cascade. chat_sessions.py's ceiling is lowered from 2090 to 1953; no ceiling was raised.
• Two new test files, 21 cases:
  • owner delete removes the dependents
  • a non-owner gets 403
  • an unknown id, or no manager yet, gets 404
  • orphan discovery, and dry run versus real cleanup
  • a non-admin gets 403 on the orphan routes
  • the cascade runs on session delete
  • the cleanup tuple has the expected shape
• Known gap, filed as bug(chat): session delete drops the chat-knowledge cleanup result from its response #16502: SessionDeleteData has no field for the new result, so the response drops it. schemas_chat.py is exactly at its ceiling. The cleanup itself happens and is tested at the backend-state level.

Verification

• Static checks only. flake8, black, isort, py_compile and the full pre-commit suite (including the file-size guard) passed, and the pre-push hook's own test selection passed. No tests were run by hand. CI is the first full run of the 21 new cases.
• Collision sweep: none of the other open PRs touches chat_knowledge*.py, chat.py or chat_sessions.py. Other PRs edit different ceiling entries in the shared size-baseline files.
• AC mapping:
  • AC1: the owner, admin and 403 tests in chat_knowledge_delete_test.py.
  • AC2: the cascade test in chat_sessions_delete_cleanup_test.py.
  • AC3: the orphan discovery and dry-run versus cleanup tests.

Model Used

Claude Opus 5 (coordinator session), with an implementation subagent.

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 54304890-5457-4e98-91e3-7bd5872dc683

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

[github-actions]

AutoBot Phase Validation Results

System Maturity: 96.5%

Phase Status:

PASS Phase 1: Core Infrastructure: 100.0%
PASS Phase 2: Knowledge Base and Memory: 100.0%
IN PROGRESS Phase 3: LLM Integration: 66.7%
PASS Phase 4: Security and Authentication: 100.0%
PASS Phase 5: Agent Orchestration: 100.0%
PASS Phase 6: Enhanced UI/UX: 100.0%
PASS Phase 7: Testing and Validation: 100.0%
PASS Phase 8: Advanced Features: 100.0%
PASS Phase 9: Multi-Modal AI: 100.0%
PASS Phase 10: Production Readiness: 100.0%

Recommendations:

• 🟡 MEDIUM: Phase 3: LLM Integration requires attention (66.7% complete): Review and implement
• ✅ System is production-ready - consider advanced features and scaling: Review and implement

[github-actions]

Notice: 29 open PRs — past the runaway threshold (25)

There is no PR queue limit, and this is not a request to defer this PR. Work proceeds one issue at a time without a cap on open PRs; review capacity is the constraint.

This notice only means the count is high enough to be worth a glance for a runaway — something opening PRs in a loop, or a merge pipeline that has stalled so nothing is draining.

Currently open:

• docs(infra): record redis role tools as binary names, not units (#16060, #16071) #16511 docs(infra): record redis role tools as binary names, not units (Redis is managed by two unit names that do not exist, and a failed restore-stop is silent #16060, redis naming: five more sites name a unit the fleet does not have (#16060 follow-up) #16071)
• refactor(lifespan): split cleanup_services and _init_graph_rag_service into steps under the length limit (#16250) #16509 refactor(lifespan): split cleanup_services and _init_graph_rag_service into steps under the length limit (function-length-check pre-commit hook blocks any commit to lifespan.py: cleanup_services (267 lines) and _init_graph_rag_service (68 lines) exceed the 65-line ceiling #16250)
• security(routers): gate knowledge_search, knowledge_search_aggregator, knowledge_suggestions and voice_stream (#16507) #16508 security(routers): gate knowledge_search, knowledge_search_aggregator, knowledge_suggestions and voice_stream (security(routers): gate knowledge_search, knowledge_search_aggregator, knowledge_suggestions and voice_stream #16507)
• test(user-management): a user in org A gets no results from org B (#16279) #16506 test(user-management): a user in org A gets no results from org B (security(users): /user-management/users/search answers anonymous callers with names from every organisation #16279)
• security(settings): record the real caller, not the literal "admin" (#16501) #16504 security(settings): record the real caller, not the literal "admin" (security(settings): sync_config and update_hardware_priority record the literal "admin" instead of the real caller #16501)
• fix(slm-frontend): give the service-log fetch one path and its real timeout (#16256) #16500 fix(slm-frontend): give the service-log fetch one path and its real timeout (tech-debt(slm-frontend): the two log viewers fetch the same journal through two paths with different timeouts #16256)
• fix(a2a): cooperative cancellation checkpoints in execute_a2a_task (#16174) #16498 fix(a2a): cooperative cancellation checkpoints in execute_a2a_task (Cancelling an A2A task does not stop its executor, so claims can only lapse rather than release #16174)
• fix(sdk): TypeScript SDK request/response parity + missing /api prefix (#15528, #16495) #16497 fix(sdk): TypeScript SDK request/response parity + missing /api prefix (bug(sdk): the TypeScript SDK still sends the parameters and parses the envelopes the Python SDK stopped sending #15528, bug(sdk): every TypeScript SDK request omits the /api prefix — 404 against a real backend #16495)
• refactor(agents): extract NPUCodeSearchAgent's pattern helpers, add declared_scopes (#16173) #16496 refactor(agents): extract NPUCodeSearchAgent's pattern helpers, add declared_scopes (NPUCodeSearchAgent cannot declare its write scopes: its file is at the size ceiling #16173)
• fix(frontend): converge the permission vocabulary onto the backend's real Permission enum, wire in v-permission (#16243) #16493 fix(frontend): converge the permission vocabulary onto the backend's real Permission enum, wire in v-permission (tech-debt(frontend): usePermissions' permission strings share nothing with the backend's Permission vocabulary #16243)
• security(research): treat fetched repos and pages as untrusted data (#16488) #16492 security(research): treat fetched repos and pages as untrusted data (security(research): treat fetched repos and pages as untrusted data: prompt-injection hardening for research, adopt and web-fetch #16488)
• feat(frontend): wire router meta.minRole into a real guard, ranked against the backend's role order (#16244) #16489 feat(frontend): wire router meta.minRole into a real guard, ranked against the backend's role order (tech-debt(frontend): router meta.minRole is declared and read by no guard #16244)
• chore(git): rename Dev_new_gui to main and main to release across the repo (#16479) #16487 chore(git): rename Dev_new_gui to main and main to release across the repo (infra(git): swap branch names across the repo for the rename (in-repo half of #16461) #16479)
• feat(secrets): wire in SecretAuditLog, consolidate SecretVault into SecretsManager (#16485) #16486 feat(secrets): wire in SecretAuditLog, consolidate SecretVault into SecretsManager (wire in SecretAuditLog, consolidate SecretVault into SecretsManager #16485)
• fix(frontend): retire four dead settings API paths, stop a fifth calling admin-gated settings from non-admin surfaces (#16481) #16484 fix(frontend): retire four dead settings API paths, stop a fifth calling admin-gated settings from non-admin surfaces (retire useSettingsApi/SystemRepository/services-api.ts/CacheService.ts dead settings paths #16481)
• feat(llm): cross-worker quota headroom store, persist real 429 headroom #16483 feat(llm): cross-worker quota headroom store, persist real 429 headroom
• chore(activity-tracking): retire the terminal/file/browser tracking hooks as dead code (#16466) #16475 chore(activity-tracking): retire the terminal/file/browser tracking hooks as dead code (chore(activity-tracking): wire in or retire terminal/file/browser activity tracking + secret_usage audit (backend) #16466)
• chore(deps): bump the npm_and_yarn group across 4 directories with 2 updates #16474 chore(deps): bump the npm_and_yarn group across 4 directories with 2 updates
• feat(collaboration): surface pending invitations in the collaboration panel (#16470) #16472 feat(collaboration): surface pending invitations in the collaboration panel (impl gap: surface pending collaboration invitations in a UI #16470)
• feat(collaboration): persist activity/notification history, add invitation list+respond endpoints (#16460) #16471 feat(collaboration): persist activity/notification history, add invitation list+respond endpoints (feat(collaboration): persist activity/notification history and add invitation list+respond endpoints #16460)
• security(settings): record the real caller on /sync and /hardware-priority, and test the anonymous and cross-org refusals (#16278) #16469 security(settings): record the real caller on /sync and /hardware-priority, and test the anonymous and cross-org refusals (security(settings): the core /api/settings routes read and write system config with no authentication #16278)
• fix(migrations): port 6 orphaned tables into the canonical Alembic chain (#16464) #16467 fix(migrations): port 6 orphaned tables into the canonical Alembic chain (fix(migrations): 6 tables in the orphaned database/migrations/ chain were never applied to the live Alembic chain #16464)
• feat(collaboration): wire the collaboration UI onto the real backend (#16443) #16462 feat(collaboration): wire the collaboration UI onto the real backend (impl gap: wire in the real-time collaborative-session UI (#608 phases 5-7 / #874) — 6 components + useSessionCollaboration, zero callers #16443)
• feat(mcp): admin-configured external MCP server bridge #16458 feat(mcp): admin-configured external MCP server bridge
• security(collaboration): authenticate and authorize the session-presence WebSocket (#16455) #16456 security(collaboration): authenticate and authorize the session-presence WebSocket (security(collaboration): WS /ws/sessions/{id}/presence trusts an unauthenticated client-supplied user_id #16455)
• fix(mcp): rename autobot-backend/mcp/ off the name it shadows (#16449) #16454 fix(mcp): rename autobot-backend/mcp/ off the name it shadows (chore(mcp): autobot-backend/mcp/ shadows the real mcp PyPI SDK, making it unreachable by name #16449)
• docs(adr): supersede ADR-001's fixed VM count with role-separation placement (#15194) #16452 docs(adr): supersede ADR-001's fixed VM count with role-separation placement (docs: a fixed 6-VM topology is documented as the architecture — AutoBot scales from one VM or Docker to whatever is needed, and ADR-001 enshrines the wrong invariant #15194)
• fix(tooling): scope protect-files.sh's secrets/ rule to real credential storage (#16446) #16448 fix(tooling): scope protect-files.sh's secrets/ rule to real credential storage (fix(tooling): protect-files.sh's secrets/ rule blocks legitimate frontend source, zero real targets #16446)
• docs: describe role-based placement instead of a fixed VM count (#15194) #16445 docs: describe role-based placement instead of a fixed VM count (docs: a fixed 6-VM topology is documented as the architecture — AutoBot scales from one VM or Docker to whatever is needed, and ADR-001 enshrines the wrong invariant #15194)

If the queue is draining normally, ignore this. Otherwise:

1. Check whether CI is dispatching at all — see the ci-dispatch-watchdog status on these PRs
2. Merge the ones whose CI has finished and review has passed: gh pr merge <number> --squash --delete-branch
3. Look for a loop opening near-identical PRs

Warn-only runaway detector — .github/workflows/pr-queue-gate.yml. It never blocks a merge.

[mrveiss]

Review of 4ed6dd5b5: approve. 0 behind, 0 trailers, closingIssuesReferences matches body ([16490]).

New api/chat_knowledge_delete.py, split out to stay under the file-size ceiling (same pattern as #15160). DELETE /context/{chat_id} checks 404 (via peek_chat_knowledge_manager, never the constructing accessor) before authorization, with a specifically-cited reason: validate_chat_ownership silently claims an unowned chat_id on its legacy-migration path, so authorizing before the existence check would return "authorized, nothing to delete" for a probe against a chat_id nothing created, instead of a clean 404. Reuses the existing ownership check rather than a new authz path. Chat-deletion cascade lives in chat_sessions_delete_cleanup.py, called from chat_sessions.py's existing delete route.

All three ACs have dedicated tests: owner-delete-cascades (test_owner_deletes_and_every_dependent_record_is_gone) plus non-owner-403 and unknown/no-manager-404; chat-deletion cascade (test_deletes_the_session_context_and_reports_it, test_the_context_is_gone_after_cleanup_runs); orphan sweep with dry-run vs real delete and non-admin-403 on both orphan routes.

[github-actions]

AutoBot Phase Validation Results

System Maturity: 96.5%

Phase Status:

PASS Phase 1: Core Infrastructure: 100.0%
PASS Phase 2: Knowledge Base and Memory: 100.0%
IN PROGRESS Phase 3: LLM Integration: 66.7%
PASS Phase 4: Security and Authentication: 100.0%
PASS Phase 5: Agent Orchestration: 100.0%
PASS Phase 6: Enhanced UI/UX: 100.0%
PASS Phase 7: Testing and Validation: 100.0%
PASS Phase 8: Advanced Features: 100.0%
PASS Phase 9: Multi-Modal AI: 100.0%
PASS Phase 10: Production Readiness: 100.0%

Recommendations:

• 🟡 MEDIUM: Phase 3: LLM Integration requires attention (66.7% complete): Review and implement
• ✅ System is production-ready - consider advanced features and scaling: Review and implement

[mrveiss]

Delta 4ed6dd5b5..f9f0b3290: approve. The only file whose change differs from the approved net diff is autobot-backend/api/chat_sessions_delete_cleanup.py:

• The new check: before the delete, _cleanup_conversation_transcript resolves both PATH.DATA_DIR / "conversation_transcripts" and transcript_path with os.path.realpath and requires real.startswith(root + os.sep). Otherwise it raises ValueError("Invalid session ID"). It then removes real.
• The same root: transcript_path comes from validate_relative_path(..., PATH.DATA_DIR / "conversation_transcripts", ...), so legitimate deletes still pass. A symlink inside the directory that points outside it is now refused, which is stricter.
• The same error handling: the new raise sits inside the same try as the existing "Invalid session ID" raise, so the function's except Exception catches it and it becomes the cleanup result, exactly like the old refusal.
• The one behaviour change, as stated: a name that resolves to the directory itself is refused up front. os.remove would refuse a directory anyway.

The CodeQL result on this head was not in yet when I posted, so whether CodeQL now credits the check is still unconfirmed.

[mrveiss]

CodeQL at f9f0b3290: NEUTRAL, with no annotations. This settles the point my delta approval left open. At the earlier head, CodeQL flagged two new py/path-injection alerts at api/chat_sessions_delete_cleanup.py:180-181. At this head it reports none, so it now credits the in-scope os.path.realpath + startswith(root + os.sep) check beside the delete. The only note in the check-run is "2 configurations not found", which is the PR matrix analysing only the languages this change touches, as designed, not a finding.