chihlasm/resolutionflow
1
0
Fork 0
Code Issues 23 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

52 pre-existing backend test failures on feat/flowpilot-migration #146

New Issue
Closed
opened 2026-04-24 18:41:48 +00:00 by chihlasm · 1 comment
chihlasm commented 2026-04-24 18:41:48 +00:00
Owner
Copy Link

Summary

After fixing async-fixture loop scoping in the RLS isolation test module (conftest event_loop override removed, pytest-asyncio bumped 0.23.0 → 0.24.0), the backend integration suite was re-run end-to-end on feat/flowpilot-migration. The RLS suite now passes 35/35, but the default suite surfaced 52 failures + 3 errors out of 1068 tests (1016 passing; 35 deselected — the RLS suite).

Confirmed not a regression from the fixture/asyncio changes: grep -cE "different loop|Event loop is closed|got Future" against the pytest log returns 0. All failures are pre-existing application-level issues on this branch.

Dominant root causes

1. Missing account_id on INSERT — RLS write-path regressions (largest bucket)

Tenant-scoped tables now enforce account_id NOT NULL (post Phase-4 RLS), but several service/endpoint code paths still create ORM objects without passing it. Affected relations observed in the log:

  • trees, sessions, ai_session_steps, maintenance_schedules, script_templates, session_resolution_outputs, tree_categories

This matches the exact pattern CLAUDE.md warns about: "Backfill migrations adding account_id: grep ALL ModelClass( sites in service code to verify account_id= is passed."

Tests affected (~25):

  • test_branch_manager.py::test_create_fork / test_switch_branch / test_get_branch_tree
  • test_session_branches_api.py::test_create_fork / test_switch_branch
  • test_session_resolutions_api.py::test_edit_output_api
  • test_session_suggested_fixes_api.py::test_record_decision_persists_and_bumps_state_version
  • test_resolution_outputs.py::test_generate_all_creates_three_outputs
  • test_trees.py::TestTrees::test_delete_tree_cleans_up_folder_and_tag_assignments
  • test_script_builder.py::TestScriptBuilderSaveToLibrary::test_save_to_library_success
  • test_script_builder.py::TestScriptBuilderSlugCollision::test_slug_collision_appends_suffix
  • test_script_builder.py::TestScriptTemplateFilters::test_mine_filter / test_shared_filter
  • test_pdf_export.py::TestPDFExport::test_export_pdf_returns_pdf_content / test_export_pdf_with_no_supporting_data
  • test_tenant_isolation_p0.py — most failures in this file
  • test_maintenance_schedules.py::test_get_schedule_for_tree (ERROR)
  • test_sessions.py::TestSessions::test_complete_session_with_cancelled_outcome (ERROR)
  • test_tenant_isolation_p0.py::test_analytics_flow_cannot_read_other_account_tree (ERROR)

2. Auth / status-code drift (≈15)

Endpoints returning 401 where tests expect 200/422/404, or 404 where tests expect 403. Route or permission wiring probably changed:

  • test_survey.py — all 8 tests (401 instead of 200/404/409/422; CSV export also drifted)
  • test_admin.py::TestAdminEndpoints::test_change_account_role_invalid
  • test_uploads.py::test_delete_upload_forbidden_for_non_owner
  • test_session_sharing.py — 6 tests (public-share access, revoke, multiple shares, view-count, ownership guard)
  • test_ai_endpoints.py::test_quota_returns_disabled_when_no_key / test_start_returns_503_when_disabled (AI disabled-state tests)

3. PSA write-back / connection schema (4)

psa_connections.display_name is now NOT NULL but fixtures don't set it.

  • test_psa_writeback_phase4.py::test_resolve_posts_to_psa_and_verifies_status
  • test_psa_writeback_phase4.py::test_resolve_surfaces_status_verification_failure
  • test_psa_writeback_phase4.py::test_resolve_skips_status_transition_when_unconfigured
  • test_psa_writeback_phase4.py::test_escalate_posts_internal_note_to_psa

4. Migration / visibility defaulting (1)

  • test_tree_sharing.py::test_migration_defaults_visibility_to_team

Full failure list

All 52 FAILED + 3 ERROR test names 
FAILED tests/test_admin.py::TestAdminEndpoints::test_change_account_role_invalid
FAILED tests/test_ai_endpoints.py::test_quota_returns_disabled_when_no_key
FAILED tests/test_ai_endpoints.py::test_start_returns_503_when_disabled
FAILED tests/test_branch_manager.py::test_create_fork
FAILED tests/test_branch_manager.py::test_switch_branch
FAILED tests/test_branch_manager.py::test_get_branch_tree
FAILED tests/test_pdf_export.py::TestPDFExport::test_export_pdf_returns_pdf_content
FAILED tests/test_pdf_export.py::TestPDFExport::test_export_pdf_with_no_supporting_data
FAILED tests/test_psa_writeback_phase4.py::test_resolve_posts_to_psa_and_verifies_status
FAILED tests/test_psa_writeback_phase4.py::test_resolve_surfaces_status_verification_failure
FAILED tests/test_psa_writeback_phase4.py::test_resolve_skips_status_transition_when_unconfigured
FAILED tests/test_psa_writeback_phase4.py::test_escalate_posts_internal_note_to_psa
FAILED tests/test_resolution_outputs.py::test_generate_all_creates_three_outputs
FAILED tests/test_script_builder.py::TestScriptBuilderSaveToLibrary::test_save_to_library_success
FAILED tests/test_script_builder.py::TestScriptBuilderSlugCollision::test_slug_collision_appends_suffix
FAILED tests/test_script_builder.py::TestScriptTemplateFilters::test_mine_filter
FAILED tests/test_script_builder.py::TestScriptTemplateFilters::test_shared_filter
FAILED tests/test_session_branches_api.py::test_create_fork
FAILED tests/test_session_branches_api.py::test_switch_branch
FAILED tests/test_session_resolutions_api.py::test_edit_output_api
FAILED tests/test_session_sharing.py::TestSessionSharing::test_access_public_share
FAILED tests/test_session_sharing.py::TestSessionSharing::test_access_revoked_share_returns_404
FAILED tests/test_session_sharing.py::TestSessionSharing::test_access_nonexistent_share_returns_404
FAILED tests/test_session_sharing.py::TestSessionSharing::test_multiple_shares_per_session
FAILED tests/test_session_sharing.py::TestSessionSharing::test_share_view_count_increments
FAILED tests/test_session_sharing.py::TestSessionSharing::test_share_requires_session_ownership
FAILED tests/test_session_suggested_fixes_api.py::test_record_decision_persists_and_bumps_state_version
FAILED tests/test_survey.py::test_submit_survey_success
FAILED tests/test_survey.py::test_submit_survey_anonymous
FAILED tests/test_survey.py::test_submit_survey_missing_responses
FAILED tests/test_survey.py::test_submit_survey_empty_body
FAILED tests/test_survey.py::test_check_invite_status_not_found
FAILED tests/test_survey.py::test_submit_with_completed_token_returns_409
FAILED tests/test_survey.py::test_list_survey_responses_admin
FAILED tests/test_survey.py::test_export_survey_responses_csv
FAILED tests/test_tenant_isolation_p0.py::test_category_tree_count_scoped_to_account
FAILED tests/test_tenant_isolation_p0.py::test_ai_session_search_cannot_see_other_users_sessions
FAILED tests/test_tenant_isolation_p0.py::test_get_tree_returns_404_not_403_for_other_account
FAILED tests/test_tenant_isolation_p0.py::test_update_tree_returns_404_not_403_for_other_account
FAILED tests/test_tenant_isolation_p0.py::test_get_session_returns_404_not_403_for_other_user
FAILED tests/test_tenant_isolation_p0.py::test_ai_session_get_returns_404_not_403_for_other_user
FAILED tests/test_tenant_isolation_p0.py::test_ai_session_retry_psa_push_requires_ownership
FAILED tests/test_tenant_isolation_p0.py::test_upload_url_returns_404_not_403_for_other_account
FAILED tests/test_tenant_isolation_p0.py::test_share_revoke_returns_404_not_403_for_other_user
FAILED tests/test_tenant_isolation_p0.py::test_cannot_access_other_account_step
FAILED tests/test_tenant_isolation_p0.py::test_cannot_access_other_account_tag
FAILED tests/test_tenant_isolation_p0.py::test_cannot_access_other_account_step_category
FAILED tests/test_tenant_isolation_p0.py::test_maintenance_schedule_returns_404_for_other_team
FAILED tests/test_tenant_isolation_p0.py::test_get_documentation_returns_404_for_other_user_session
FAILED tests/test_tree_sharing.py::test_migration_defaults_visibility_to_team
FAILED tests/test_trees.py::TestTrees::test_delete_tree_cleans_up_folder_and_tag_assignments
FAILED tests/test_uploads.py::test_delete_upload_forbidden_for_non_owner
ERROR tests/test_maintenance_schedules.py::test_get_schedule_for_tree
ERROR tests/test_sessions.py::TestSessions::test_complete_session_with_cancelled_outcome
ERROR tests/test_tenant_isolation_p0.py::test_analytics_flow_cannot_read_other_account_tree

Suggested approach

Tackle the buckets in order — bucket 1 is the biggest lever and most mechanical:

  1. Sweep service/endpoint code for ModelClass( sites against the tables in bucket 1 and add account_id= where missing. CLAUDE.md already calls this out as the failure mode.
  2. Re-audit survey and session-sharing routes — 401s-instead-of-200 suggest these endpoints are now behind an auth guard that wasn't there before. Decide whether the tests or the routes are the source of truth.
  3. Update PSA-connection fixtures/tests for the display_name NOT NULL column.
  4. Audit test_ai_endpoints.py disabled-state tests against current AI feature-flag wiring.

Reproduction

docker exec resolutionflow_postgres psql -U postgres -c "DROP DATABASE IF EXISTS resolutionflow_test"
docker exec resolutionflow_postgres psql -U postgres -c "CREATE DATABASE resolutionflow_test"
docker exec -i -w /app resolutionflow_backend pytest --no-cov

Run on: feat/flowpilot-migration at 24972e8 (2026-04-24). Full log retained locally at /tmp/pytest-default.log on devserver01.

## Summary After fixing async-fixture loop scoping in the RLS isolation test module (conftest `event_loop` override removed, pytest-asyncio bumped 0.23.0 → 0.24.0), the backend integration suite was re-run end-to-end on `feat/flowpilot-migration`. The RLS suite now passes 35/35, but the default suite surfaced **52 failures + 3 errors out of 1068 tests** (1016 passing; 35 deselected — the RLS suite). Confirmed not a regression from the fixture/asyncio changes: `grep -cE "different loop|Event loop is closed|got Future"` against the pytest log returns 0. All failures are pre-existing application-level issues on this branch. ## Dominant root causes ### 1. Missing `account_id` on INSERT — RLS write-path regressions (largest bucket) Tenant-scoped tables now enforce `account_id NOT NULL` (post Phase-4 RLS), but several service/endpoint code paths still create ORM objects without passing it. Affected relations observed in the log: - `trees`, `sessions`, `ai_session_steps`, `maintenance_schedules`, `script_templates`, `session_resolution_outputs`, `tree_categories` This matches the exact pattern CLAUDE.md warns about: *"Backfill migrations adding `account_id`: grep ALL `ModelClass(` sites in service code to verify `account_id=` is passed."* **Tests affected (~25):** - `test_branch_manager.py::test_create_fork` / `test_switch_branch` / `test_get_branch_tree` - `test_session_branches_api.py::test_create_fork` / `test_switch_branch` - `test_session_resolutions_api.py::test_edit_output_api` - `test_session_suggested_fixes_api.py::test_record_decision_persists_and_bumps_state_version` - `test_resolution_outputs.py::test_generate_all_creates_three_outputs` - `test_trees.py::TestTrees::test_delete_tree_cleans_up_folder_and_tag_assignments` - `test_script_builder.py::TestScriptBuilderSaveToLibrary::test_save_to_library_success` - `test_script_builder.py::TestScriptBuilderSlugCollision::test_slug_collision_appends_suffix` - `test_script_builder.py::TestScriptTemplateFilters::test_mine_filter` / `test_shared_filter` - `test_pdf_export.py::TestPDFExport::test_export_pdf_returns_pdf_content` / `test_export_pdf_with_no_supporting_data` - `test_tenant_isolation_p0.py` — most failures in this file - `test_maintenance_schedules.py::test_get_schedule_for_tree` (ERROR) - `test_sessions.py::TestSessions::test_complete_session_with_cancelled_outcome` (ERROR) - `test_tenant_isolation_p0.py::test_analytics_flow_cannot_read_other_account_tree` (ERROR) ### 2. Auth / status-code drift (≈15) Endpoints returning `401` where tests expect `200`/`422`/`404`, or `404` where tests expect `403`. Route or permission wiring probably changed: - `test_survey.py` — all 8 tests (401 instead of 200/404/409/422; CSV export also drifted) - `test_admin.py::TestAdminEndpoints::test_change_account_role_invalid` - `test_uploads.py::test_delete_upload_forbidden_for_non_owner` - `test_session_sharing.py` — 6 tests (public-share access, revoke, multiple shares, view-count, ownership guard) - `test_ai_endpoints.py::test_quota_returns_disabled_when_no_key` / `test_start_returns_503_when_disabled` (AI disabled-state tests) ### 3. PSA write-back / connection schema (4) `psa_connections.display_name` is now NOT NULL but fixtures don't set it. - `test_psa_writeback_phase4.py::test_resolve_posts_to_psa_and_verifies_status` - `test_psa_writeback_phase4.py::test_resolve_surfaces_status_verification_failure` - `test_psa_writeback_phase4.py::test_resolve_skips_status_transition_when_unconfigured` - `test_psa_writeback_phase4.py::test_escalate_posts_internal_note_to_psa` ### 4. Migration / visibility defaulting (1) - `test_tree_sharing.py::test_migration_defaults_visibility_to_team` ## Full failure list <details> <summary>All 52 FAILED + 3 ERROR test names</summary> ``` FAILED tests/test_admin.py::TestAdminEndpoints::test_change_account_role_invalid FAILED tests/test_ai_endpoints.py::test_quota_returns_disabled_when_no_key FAILED tests/test_ai_endpoints.py::test_start_returns_503_when_disabled FAILED tests/test_branch_manager.py::test_create_fork FAILED tests/test_branch_manager.py::test_switch_branch FAILED tests/test_branch_manager.py::test_get_branch_tree FAILED tests/test_pdf_export.py::TestPDFExport::test_export_pdf_returns_pdf_content FAILED tests/test_pdf_export.py::TestPDFExport::test_export_pdf_with_no_supporting_data FAILED tests/test_psa_writeback_phase4.py::test_resolve_posts_to_psa_and_verifies_status FAILED tests/test_psa_writeback_phase4.py::test_resolve_surfaces_status_verification_failure FAILED tests/test_psa_writeback_phase4.py::test_resolve_skips_status_transition_when_unconfigured FAILED tests/test_psa_writeback_phase4.py::test_escalate_posts_internal_note_to_psa FAILED tests/test_resolution_outputs.py::test_generate_all_creates_three_outputs FAILED tests/test_script_builder.py::TestScriptBuilderSaveToLibrary::test_save_to_library_success FAILED tests/test_script_builder.py::TestScriptBuilderSlugCollision::test_slug_collision_appends_suffix FAILED tests/test_script_builder.py::TestScriptTemplateFilters::test_mine_filter FAILED tests/test_script_builder.py::TestScriptTemplateFilters::test_shared_filter FAILED tests/test_session_branches_api.py::test_create_fork FAILED tests/test_session_branches_api.py::test_switch_branch FAILED tests/test_session_resolutions_api.py::test_edit_output_api FAILED tests/test_session_sharing.py::TestSessionSharing::test_access_public_share FAILED tests/test_session_sharing.py::TestSessionSharing::test_access_revoked_share_returns_404 FAILED tests/test_session_sharing.py::TestSessionSharing::test_access_nonexistent_share_returns_404 FAILED tests/test_session_sharing.py::TestSessionSharing::test_multiple_shares_per_session FAILED tests/test_session_sharing.py::TestSessionSharing::test_share_view_count_increments FAILED tests/test_session_sharing.py::TestSessionSharing::test_share_requires_session_ownership FAILED tests/test_session_suggested_fixes_api.py::test_record_decision_persists_and_bumps_state_version FAILED tests/test_survey.py::test_submit_survey_success FAILED tests/test_survey.py::test_submit_survey_anonymous FAILED tests/test_survey.py::test_submit_survey_missing_responses FAILED tests/test_survey.py::test_submit_survey_empty_body FAILED tests/test_survey.py::test_check_invite_status_not_found FAILED tests/test_survey.py::test_submit_with_completed_token_returns_409 FAILED tests/test_survey.py::test_list_survey_responses_admin FAILED tests/test_survey.py::test_export_survey_responses_csv FAILED tests/test_tenant_isolation_p0.py::test_category_tree_count_scoped_to_account FAILED tests/test_tenant_isolation_p0.py::test_ai_session_search_cannot_see_other_users_sessions FAILED tests/test_tenant_isolation_p0.py::test_get_tree_returns_404_not_403_for_other_account FAILED tests/test_tenant_isolation_p0.py::test_update_tree_returns_404_not_403_for_other_account FAILED tests/test_tenant_isolation_p0.py::test_get_session_returns_404_not_403_for_other_user FAILED tests/test_tenant_isolation_p0.py::test_ai_session_get_returns_404_not_403_for_other_user FAILED tests/test_tenant_isolation_p0.py::test_ai_session_retry_psa_push_requires_ownership FAILED tests/test_tenant_isolation_p0.py::test_upload_url_returns_404_not_403_for_other_account FAILED tests/test_tenant_isolation_p0.py::test_share_revoke_returns_404_not_403_for_other_user FAILED tests/test_tenant_isolation_p0.py::test_cannot_access_other_account_step FAILED tests/test_tenant_isolation_p0.py::test_cannot_access_other_account_tag FAILED tests/test_tenant_isolation_p0.py::test_cannot_access_other_account_step_category FAILED tests/test_tenant_isolation_p0.py::test_maintenance_schedule_returns_404_for_other_team FAILED tests/test_tenant_isolation_p0.py::test_get_documentation_returns_404_for_other_user_session FAILED tests/test_tree_sharing.py::test_migration_defaults_visibility_to_team FAILED tests/test_trees.py::TestTrees::test_delete_tree_cleans_up_folder_and_tag_assignments FAILED tests/test_uploads.py::test_delete_upload_forbidden_for_non_owner ERROR tests/test_maintenance_schedules.py::test_get_schedule_for_tree ERROR tests/test_sessions.py::TestSessions::test_complete_session_with_cancelled_outcome ERROR tests/test_tenant_isolation_p0.py::test_analytics_flow_cannot_read_other_account_tree ``` </details> ## Suggested approach Tackle the buckets in order — bucket 1 is the biggest lever and most mechanical: 1. **Sweep service/endpoint code for `ModelClass(` sites** against the tables in bucket 1 and add `account_id=` where missing. CLAUDE.md already calls this out as the failure mode. 2. **Re-audit survey and session-sharing routes** — 401s-instead-of-200 suggest these endpoints are now behind an auth guard that wasn't there before. Decide whether the tests or the routes are the source of truth. 3. **Update PSA-connection fixtures/tests** for the `display_name` NOT NULL column. 4. **Audit `test_ai_endpoints.py` disabled-state tests** against current AI feature-flag wiring. ## Reproduction ```bash docker exec resolutionflow_postgres psql -U postgres -c "DROP DATABASE IF EXISTS resolutionflow_test" docker exec resolutionflow_postgres psql -U postgres -c "CREATE DATABASE resolutionflow_test" docker exec -i -w /app resolutionflow_backend pytest --no-cov ``` Run on: `feat/flowpilot-migration` at `24972e8` (2026-04-24). Full log retained locally at `/tmp/pytest-default.log` on devserver01.
chihlasm commented 2026-05-01 04:57:14 +00:00
Author
Owner
Copy Link

Closing — all 52 failures resolved on main.

Verified 2026-05-01 by running every module called out in this issue (test_tenant_isolation_p0, test_survey, test_psa_writeback_phase4, test_branch_manager, test_session_branches_api, test_session_resolutions_api, test_session_sharing, test_session_suggested_fixes_api, test_resolution_outputs, test_script_builder, test_pdf_export, test_trees, test_uploads, test_admin, test_ai_endpoints, test_tree_sharing, test_maintenance_schedules, test_sessions) — 240/240 passed.

Key fixes that landed:

  • 0d9babb — account_id on AISessionStep creations
  • b18072e — account_id on PsaMemberMapping
  • d6218f2 — import all models in conftest so create_all sees the full schema
  • Phase-4 RLS sweep commits (64f004a, f9248ae, c6da4eb, 893b8a5, etc.)

feat/flowpilot-migration was merged into main.

Closing — all 52 failures resolved on main. Verified 2026-05-01 by running every module called out in this issue (`test_tenant_isolation_p0`, `test_survey`, `test_psa_writeback_phase4`, `test_branch_manager`, `test_session_branches_api`, `test_session_resolutions_api`, `test_session_sharing`, `test_session_suggested_fixes_api`, `test_resolution_outputs`, `test_script_builder`, `test_pdf_export`, `test_trees`, `test_uploads`, `test_admin`, `test_ai_endpoints`, `test_tree_sharing`, `test_maintenance_schedules`, `test_sessions`) — **240/240 passed**. Key fixes that landed: - `0d9babb` — `account_id` on AISessionStep creations - `b18072e` — `account_id` on PsaMemberMapping - `d6218f2` — import all models in conftest so `create_all` sees the full schema - Phase-4 RLS sweep commits (`64f004a`, `f9248ae`, `c6da4eb`, `893b8a5`, etc.) `feat/flowpilot-migration` was merged into main.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
10x-analysis
From 10x product analysis
 backend
FastAPI/Python work
 bug
Something isn't working
 collaboration
Team sharing, escalation, notifications
 database
PostgreSQL/schema changes
 documentation
Documentation updates
 duplicate
This issue or pull request already exists
 enhancement
Improvement to existing feature
 feature
New functionality
 frontend
React work
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 integration
PSA, external service integrations
 intelligence
Analytics, AI, data-driven features
 invalid
This doesn't seem right
 priority: high
Do this soon
 priority: low
Nice to have
 priority: medium
Important but not urgent
 question
Further information is requested
 quick-win
Low effort, high value
 strategic
Long-term strategic bet
 ux
User experience improvements
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
chihlasm
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: chihlasm/resolutionflow#146
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?