feat: add project migration tests and update documentation#1330
Conversation
Core handler tests: - Add test_list_projects_with_tenant_header_scopes_results to verify X-Tenant-ID scoping on GET /api/v1/projects (org-filtered + NULL rows vs. unscoped list returning all) Orchestrator association tests (new file): - crates/orchestrator/tests/project_associations_http.rs: 9 tests covering POST/DELETE agent and workflow associations, 404 on missing agent/workflow, and behavior when project ID does not exist in core CLI integration tests: - Add project_tests module to crates/cli/tests/integration_test.rs with mockito tests verifying CRUD routes to core URL, association routes to orchestrator URL, and delete_project pre-check behavior Documentation: - README: add agentd-core to port table (17000/7000), update description to mention project management, add project management CLI entry, add v0.15.0 migration status section with operator upgrade note - docs/storage.md: add service-to-table ownership table with note that projects moved from orchestrator to core in v0.15.0 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
geoffjay
left a comment
There was a problem hiding this comment.
Review: feat: add project migration tests and update documentation
Stack position: epic-1314 is stacked directly on feature/projects-to-core (#1325). Two commits sit above the base in this branch — both scoped to tests and docs. The implementation (including with_core_url, the delete_project association pre-check, and the core-routing helpers) is already present in the base, so the tests compile and exercise live code. This is the correct final PR to merge in the series.
Note on PR #1328: The delete_project association pre-check IS implemented correctly here (list_project_agents then list_project_workflows, bail early if either is non-zero, then delete_core). The needs-rework on #1328 can be narrowed to just fixing the now-stale docstring — the safety concern is resolved.
Approved — non-blocking suggestions below
Non-blocking: no test for delete_project blocked by workflows (only agents case is covered)
test_delete_project_blocked_when_agents_associated exercises the agents guard. There is no symmetric test for when ONLY workflows are still associated. The implementation calls list_project_agents first and short-circuits on agents > 0, so the workflow guard is never reached in that test — but the workflow guard itself is untested. A minimal complement:
#[tokio::test]
async fn test_delete_project_blocked_when_workflows_associated() {
let core_server = Server::new_async().await;
let mut orch_server = Server::new_async().await;
let project_id = Uuid::new_v4();
let wf_id = Uuid::new_v4();
// Agents: none
orch_server
.mock("GET", format!("/projects/{project_id}/agents").as_str())
.with_status(200)
.with_body(json!({ "items": [], "total": 0, "limit": 50, "offset": 0 }).to_string())
.with_header("content-type", "application/json")
.create_async().await;
// Workflows: one remaining
orch_server
.mock("GET", format!("/projects/{project_id}/workflows").as_str())
.with_status(200)
.with_body(json!({
"items": [{ "id": wf_id, "name": "busy-wf" }],
"total": 1, "limit": 50, "offset": 0
}).to_string())
.with_header("content-type", "application/json")
.create_async().await;
let client = OrchestratorClient::new(orch_server.url()).with_core_url(core_server.url());
let result = client.delete_project(&project_id).await;
assert!(result.is_err());
let msg = result.unwrap_err().to_string();
assert!(msg.contains("workflow") || msg.contains("associated"), "{msg}");
}Non-blocking: project_id FK language in docs/storage.md is technically inaccurate
The table says:
| `agents` | agentd-orchestrator | `agentd/agent.db` | `project_id` FK points to core `projects.id` |
These are separate SQLite databases on separate services — there is no database-enforced foreign key constraint. "FK" implies referential integrity that does not exist here. Consider phrasing it as a logical reference instead:
project_idreferences coreprojects.id(logical ref — no DB-enforced constraint)
Non-blocking: agentd-index is still in the README (pre-existing, out of scope)
CLAUDE.md documents that the index crate was removed as an experiment. Lines 29 and 373 in README.md still list agentd-index. This PR is already editing the port table and service list — a good opportunity to drop those stale rows — but it is not required for approval.
Everything else looks good
- Core handler test (
test_list_projects_with_tenant_header_scopes_results) — correctly seeds 3 projects with different org scoping and asserts thatX-Tenant-ID: org-xreturns 2 (matching + NULL-org rows), while a headerless request returns 3. Matches thelist_org()storage contract precisely. - Orchestrator association tests (9 tests) —
NullBackendis comprehensive,build_app()is clean and reusable, agent names are unique per test to prevent parallel-test flakiness, andtest_associate_agent_nonexistent_project_still_succeedsexplicitly documents the deliberate lack of cross-service project validation with a clear comment. - CLI routing tests — two-server mockito setup cleanly separates core vs. orchestrator traffic.
test_delete_project_checks_associations_then_deletes_from_coreasserts all three mocks (agents list, workflows list, core DELETE) viaassert_async(), which is exactly right. - Documentation — port table addition, orchestrator description update, v0.15.0 migration status block, and the
backfill-projectsoperator note are all accurate and match the implementation.
Implements the Phase 5 cleanup for the project entity migration (#1305) — integration tests, association endpoint tests, CLI routing tests, and documentation updates.
Core handler tests (
crates/core/src/api/projects.rs):test_list_projects_with_tenant_header_scopes_results— verifiesX-Tenant-IDheader scoping onGET /api/v1/projects(org-filtered results include matching + NULL-org rows; no header returns all)Orchestrator association tests (new
crates/orchestrator/tests/project_associations_http.rs):POST/DELETEagent and workflow associations, 404 on missing agent/workflow, and documented behavior that the orchestrator does not validate project IDs against coreCLI integration tests (
crates/cli/tests/integration_test.rs):project_testsmodule with 6 mockito tests verifying: CRUD methods route to core URL, association methods route to orchestrator URL,delete_projectissues association pre-checks before deleting from core, anddelete_projectreturns an error when agents are still associatedDocumentation:
README.md: addagentd-coreto port table (17000/7000), update core description to mention project management, add project management CLI entry, add v0.15.0 project entity migration status section with operator upgrade notedocs/storage.md: add service-to-table ownership table notingprojectsmoved from orchestrator to core in v0.15.0Closes #1314