Skip to content

refactor: remove 50 backend and scheduler symbols no production code references (#3433) - #3472

Merged
vybe merged 12 commits into
devfrom
feature/3433-dead-symbols
Oct 11, 2026
Merged

vybe merged 12 commits into
devfrom
feature/3433-dead-symbols

Conversation

@vybe

@vybe vybe commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #3433. One of five independent stale-code PRs off dev. Draft: lands after #3440 (the scheduler process-schedule removal), which touches the same scheduler files.

What

  • 50 dead symbols removed across backend services, adapters, db modules, utils and the scheduler — every one re-verified to have no production caller on the live tree immediately before deletion (backend, scheduler, MCP server, base image, scripts, compose files, the pinned enterprise submodule, and by-name dispatch through the database facade), plus 5 symbols that became unreferenced as a result (get_active_count, update_schedule, AgentTaskMetrics, _DEFAULT_CHANNEL_TIMEOUT, the _backlog_service global) and the imports and constants only they used.
  • Kept, deliberately: distinct_failing_count and registered_columns (their docstrings declare them test/observability accessors); SshService.cleanup_agent_credentials (see Handoffs).
  • No lost call site found. The heavily tested _sanitize_filename in adapters/message_router.py was a byte-identical duplicate of the live upload_service.sanitize_filename; its 8 tests now run against the live function.
  • Tests re-pointed, not dropped, where they covered live behaviour: the freeze-policy asserts now call sync_freeze_reason(...) is not None; the bulk heartbeat test gained the payload asserts the deleted single-agent test carried; two new sync-loop tests prove a disabled schedule is never registered and is dropped when disabled between ticks (previously untested).
  • Scheduler test fixture: the same two one-line commits as refactor(scheduler): remove the process-schedule remnant (#3432) #3440 (chain_depth, claim_token).
  • Docs: the memory docs that named removed symbols are corrected, including ssh-access.md (it claimed the SSH cleanup runs at agent stop/delete; nothing calls it) and the snippets that described a scheduler client module removed in v0.7.0.

No schema, migration, route or MCP-tool change.

Rulings carried (orchestrator, on the operator's behalf — plan file)

Review + security

/review (reading review, report-only): MERGEABLE, 0 critical. Independent per-name sweep at the branch tip: zero live references, nothing exported through __all__ or a package __init__. Each security-relevant removal has live code performing the same check today (Slack signature verification in the webhook handler, validate_password_strength, inline webhook-secret decryption, the allow-list 403). /cso diff pass: no findings, net surface reduction. Fixed after review: the sync-loop disabled-schedule tests and the two doc corrections.

Tests

  • tests/scheduler_tests: 237 passed, 0 failed.
  • All 20 unit files this branch edits, shuffled (seed 12345), one per process: 659 passed, 32 skipped, 0 failed.
  • New sync-loop tests are mutation-proven (each fails when its branch is broken).
  • Import smoke: backend main (674 routes) and scheduler.main import; compileall clean.
  • git merge-tree against dev (21b4a11): clean.
  • Not run here: the full unit island (CI); two edited files under tests/integration/ need a live Redis and were collect-checked only.

Before merge

Handoffs

🤖 Generated with Claude Code

Trinity Agent (trinity) and others added 9 commits October 9, 2026 10:38
…#3432)

The hand-written fixture DDL lacked the chain_depth column that #3120 added
to SchedulerDatabase.create_execution, so every agent-path INSERT in the
scheduler tier raised OperationalError (70 failed / 173 passed). With the
column: 3 failed / 240 passed; the remaining three are a separate drift
(claim_token), left out of this fixture-only commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#3433)

The hand-written fixture DDL lacked the claim_token column (TEXT, as in
db/schema.py) that SchedulerDatabase's require_unclaimed guard (#2514)
reads, leaving three scheduler tests red:
test_1472::test_a2_dispatch_exception_advances_next_run and two in
test_async_dispatch. With the column: 243 passed, 0 failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Removes symbols with no production caller on the live tree (re-verified
over src, docker, scripts, tests, docs/memory and the enterprise
submodule immediately before deletion):

- ssh_service: cleanup_expired_keys, list_active_keys, cleanup_agent_keys
  (cleanup_agent_credentials itself is kept; see #3433 adjacent finding)
- telegram_media: process_photo, process_document (+ os/tempfile imports)
- github_service: get_github_service, GitHubAuthError, GitHubPermissionError
- monitoring_alerts: alert_stuck_execution
- subscription_auto_switch: handle_rate_limit_error
- template_service: get_name_from_template
- skill_service: _persisted_commit
- settings_service: clear_template_registry_lkg
- system_agent_service: is_registered
- slot_service: is_at_capacity + get_active_count (its only caller)
- slack_service: verify_slack_signature (+ get_slack_signing_secret import)
- heartbeat_service: heartbeat_status (T1: 2 unique tests re-pointed to
  heartbeat_status_bulk, 4 duplicates deleted)

Tests that only exercised a removed symbol are deleted; memory docs that
described them are corrected. Targeted unit files: 164 passed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Removes symbols with no production caller on the live tree (re-verified
over src, docker, scripts, tests, docs/memory and the enterprise
submodule immediately before deletion):

- dispatch_breaker: get_all_dispatch_states (live reader is
  get_dispatch_states_for; its Redis-None fail-open is already pinned by
  test_states_for_no_redis_is_all_closed)
- idempotency_service: derive_schedule_key (scheduler builds sched:{id})
- agent_mcp_key_service: build_mcp_key_env_overrides (test re-pointed to
  _mint_and_build_env)
- mcp_tool_names: compute_tool_name (3 slug tests re-pointed to
  compute_tool_names)
- agent_client.http_pool: _get_http_client (tests use _acquire_client)
- agent_call_limiter: _set_cancel_sync
- a2a_payment_gate: AllowlistRefused
- a2a_gate: get_skills_provider (T6)
- compatibility.static_checks: _is_skill_md
- git_service.trinity_files: _persistent_state_for (4 fallback tests
  re-pointed to _read_trinity_yaml_list over the persistent-state file)
- skill_sets: split_set_ref
- event_bus: _serialize
- backlog_service: get_backlog_service + the _backlog_service global

redelivery_governor.distinct_failing_count is kept (T2: declared
test/observability accessor). Targeted unit files: 374 passed, 32 skipped
(lupa-less fakeredis + a pre-existing #1609 skip).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oint B (#3433)

Removes symbols with no production caller on the live tree (re-verified
over src, docker, scripts, tests, docs/memory and the enterprise
submodule; DatabaseManager delegates by name, no __getattr__ passthrough):

- message_router: _get_channel_timeout + _DEFAULT_CHANNEL_TIMEOUT (caller
  removed on purpose in #221; the channel_timeout_seconds setting has no
  other reader, and the docs now say so), _format_file_size,
  _sanitize_filename (+ _FILENAME_MAX_LENGTH, _FILENAME_SAFE_CHARS_RE and
  the os/unicodedata/format_file_size imports only it used). The copy was
  code-identical to upload_service.sanitize_filename, the live path; its
  8 tests are re-pointed there (T4), as is the file-size test.
- voip_audio: pop_frames
- db.schedules.webhooks: set_webhook_enabled, _decrypt_webhook_secret
- db.slack_channels: unbind_channel
- db.telegram_channels: delete_groups_for_binding
- db.role_readiness: delete_role_readiness (+ the delete import)
- db.agent_cleanup: find_orphan_agent_names
- utils.password_validation: is_password_weak
- scheduler database: should_freeze_schedules (T3: asserts re-pointed to
  sync_freeze_reason)

agent_cleanup.registered_columns is kept (T2: parity-guard accessor).
Targeted unit files: 146 passed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Removes scheduler symbols with no production caller (re-verified over
src, docker, scripts, compose, tests, docs/memory and the enterprise
submodule; the backend's update_schedule and asyncio's loop.run_forever
are unrelated same-name symbols):

- service: add_schedule, remove_schedule, update_schedule,
  reload_schedules (the dead push-API section; the scheduler syncs by
  polling) and run_forever. The container runs
  `python -m scheduler.main` -> SchedulerApp.start() ->
  _run_until_shutdown(), which carries its own heartbeat + sync loop; no
  compose command override, no script calls it.
- database: get_recent_executions
- locking: is_schedule_locked
- models: TriggerSource, AgentTaskResponse + AgentTaskMetrics

Tests: add/remove job tests re-pointed to _add_job/_remove_job; the
disabled-schedule test is deleted with add_schedule (its enabled gate
existed only there); update/reload tests and the reload reminder-reclaim
test deleted (reclaim stays covered via _sync_schedules/_reconcile_reminders);
the skipped-execution probe now reads the row with a direct SELECT.
No process-schedule symbol is touched (#3432 owns those);
reload_schedules is deleted whole, so a textual conflict with #3432's
cut inside it resolves by taking the deletion (T5).

scheduler_tests tier: 235 passed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…est carried (#3433)

A1 deleted test_status_alive with heartbeat_status. Its unique asserts
(heartbeat_memory_mb, heartbeat_active_executions, the last-heartbeat age
bound) now live on test_bulk_mixed_states_single_round_trip, the live
path. Mutation-checked: nulling the memory_mb / active_executions
pass-through in _compute_status turns this test red.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The deleted test_add_disabled_schedule only exercised the dead
add_schedule push API. Pin the live sync path instead: a disabled row
(present at boot or inserted between ticks) never becomes a job, and a
schedule disabled between ticks is removed on the next sync.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…it is (#3433)

- ssh-access / async-docker-operations: cleanup_agent_credentials() exists
  but no stop/delete path calls it; say so instead of claiming it runs at
  agent stop/delete (follow-up bug).
- scheduling / execution-queue / model-selection / scheduler-service:
  src/scheduler/agent_client.py was removed in v0.7.0. The scheduler
  dispatches through _call_backend_execute_task -> POST
  /api/internal/execute-task; the backend's TaskExecutionService POSTs the
  agent's /api/task via agent_post_with_retry. Replace the snippets and
  drop the file-table rows that named the missing module.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
trinity-ability and others added 2 commits October 10, 2026 09:44
# Conflicts:
#	tests/scheduler_tests/conftest.py
…moval left behind (#3433)

slack_service.py kept a 'Request Verification' banner and a module-docstring
bullet for the removed verify_slack_signature (the live check is
adapters/transports/slack_webhook.py::_verify_signature); github_service.py
kept a trailing 'Factory Function' banner for the removed get_github_service.
Comment-only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vybe
vybe marked this pull request as ready for review October 11, 2026 16:37
Conflicts, both sides deleting adjacent code:
- src/scheduler/service.py: drop both the push-style schedule methods
  (this branch) and the process-schedule block (#3440).
- test_1296_reminders.py: drop the reload_schedules test (method removed here).
- test_sync_loop.py: keep both new test classes.
- scheduler-service.md: drop both sets of removed-method rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@trinity-ability trinity-ability left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed: per-name sweep finds no live reference on dev, enterprise submodule or by-name facade dispatch; 25,112 unit tests pass (the 1 failure, test_1160 concurrent boot, also fails on clean dev). Conflicts with #3440 resolved as union of deletions; scheduler suite 235 passed on the merged tree. The regression-diff failure is test_2163's 0.02s-vs-0.05s timing race in portal briefings, a file this PR does not touch; passes locally.

@vybe
vybe merged commit f96fe45 into dev Oct 11, 2026
26 of 27 checks passed
vybe pushed a commit that referenced this pull request Oct 11, 2026
execution-queue.md: both branches rewrote the scheduler's dispatch bullets;
combine into one bullet carrying #3472's dispatch path and this branch's
in-process activity tracking.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants