chore(dashboard): remove confirmed dead code in the dashboard handlers - #6941
Conversation
Four symbols in the dashboard handlers group with no reachable caller, each confirmed by three independent scans plus a route-reachability pass. - `_auto_install_agent` (handlers/agents.py): exactly one real NAME token repo-wide, and it is the re-export line in handlers/__init__.py. That module has no `__all__` -- it is a `# noqa: F401` shim -- so a name listed there with no importer is unreachable. `install_agent` keeps four other live callers, so its import stays. - `import signal` and its Windows `= None` stub (handlers/terminal.py): the whole-word name occurs on exactly those two lines; the later hits are the English word in comments. Both halves of the shim pair were dead, so removing both keeps it symmetric. - The `skipped` tally in `list_skill_tree` (handlers/_shared.py): initialised and incremented in three branches, never read -- not returned, not logged. `AugAssign` is Store-only, so the name is never Loaded. The sensitive-path guard itself is untouched; only the unread counter goes. - `_, artifact_base = _cdn_bases()` in `api_update_channel` (handlers/updates.py): neither binding is read. The sibling call site at line 1040 already uses the `feed_base, _artifact_base` convention. No test references any of the four, so nothing is deleted from test/.
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Surgical dead-code removal with verified deadness claims; the risky lookalikes (guards, config-apply wiring, CDN validation) are demonstrably untouched or live elsewhere. I independently confirmed the four deletions: [DESIGN-REVIEWED] 7baa995 |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All four deletions verify against the repo at HEAD, and nothing extra rides along. Every hunk in the patch maps to exactly one of the four declared removals; the imports each deletion touches ( First-Principles-Verdict: PASS Pure subtraction, each of the four deletions independently verified dead by counted greps, and the diff ships nothing the description does not declare. What this change shipsIntent: delete confirmed-dead code from the dashboard handlers group — a FIX (removing unearned surface), not an addition.
[FIRST-PRINCIPLES-REVIEWED] 7baa995 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
bolichen97
left a comment
There was a problem hiding this comment.
Independently verified every removed symbol (grep across full repo + PR head, string/dynamic/registry lookup checks) has zero remaining references anywhere -- no dangling handler/registry entry (the #6876 defect class). CI all green. Approving.
What
Removes four dead symbols (17 lines) from the dashboard handlers group:
src/kiro_crew/dashboard/handlers/,src/kiro_crew/dashboard/routes/,handlers_instances.py,handlers_system.py— 77 files, 2235 module-level names.No behaviour change. No test deleted: none of the four is referenced by any test.
Deleted, with evidence
1.
_auto_install_agent—handlers/agents.py:153-159+ re-exporthandlers/__init__.py:58Exactly one real NAME token repo-wide, and it is the re-export line.
handlers/__init__.pyhas no__all__— it is a# noqa: F401shim — so a name listed there with no importer is unreachable. Zero string refs, zero non-Python refs, zerogetattr/dispatch-table/importlib/entry-point refs.Not a missing wire: a dashboard agent-config change is applied to kiro-cli today by a different mechanism.
api_agent_config's PUT commits through_commit_agent_config, whose step (4) is_write_installed_config_locked(installed_path, config)— a direct write to the live spec kiro-cli reads.install_agentkeeps four live callers on the catalog-changing paths (skill install/uninstall, agent-package install/uninstall), so its import stays and there is no F401.2.
import signal+ its Windows= Nonestub —handlers/terminal.py:38, 43The whole-word name occurs on exactly those two lines. The later hits (541, 918, 1586, 1602) are the English word "signal" in comments —
tokenizeclassifies themCOMMENT, notNAME. Nosignal.attribute access anywhere in the module, on either platform branch. Since the name is never Loaded, its absence raises noNameErrorand the# type: ignore[assignment]guarded nothing. Both halves of the shim pair were dead, so removing both keeps the POSIX/Windows structure symmetric for the still-usedfcntl,_ptyandtermios.3. The
skippedtally inlist_skill_tree—handlers/_shared.py:1433, 1450, 1457, 1461Initialised and incremented in three branches, never read: not returned, not logged, not in the response.
AugAssignis a Store-only node, so the name is never Loaded.The sensitive-path guard is untouched. Every
is_sensitive_path(...)gate, the symlink-escapereal.relative_to(skill_root...)check, the exception handler, and all fourcontinuestatements are unchanged — withheld entries are still withheld. The sole caller (handlers/prompts.py:573) binds only the returned list, and this path's SEL event (prompts.py:600-604) logscount=len(entries)— the count of returned entries, never a withheld count. No response field, log line, metric or SEL event consumed the tally, and sibling listing paths report no withheld-entry count either, so no existing convention breaks.4.
_, artifact_base = _cdn_bases()—handlers/updates.py:2118Inside
api_update_channel; neither binding is read afterwards.cdn_bases()(platform/update_layout.py:167-175) readsKIROCREW_CDN_BASE, normalizes, and returns a tuple — no caching, no validation, no HTTPS pin, no governance call, so discarding the call is a no-op.Validation is a separate function (
cdn_bases_are_safe(),update_layout.py:186-196, with the pin in_SAFE_CDN_BASE_RE), and the approval-time safety gate that actually consumes it is intact atupdates.py:2309-2343—update_blocked_reason(feed_base) or update_blocked_reason(artifact_base)plus_cdn_safe(). The sibling read atupdates.py:1040already used thefeed_base, _artifact_baseconvention and is unaffected.How the candidates were found
Vulture is ~80% noise in this group: an HTTP handler has no Python caller by construction —
routes/*.pyreaches it viaapp.router.add_get(path, handlers.api_x)and the request comes fromwebsite/src. So "has a caller" was replaced with "is registered", then with "is reached".--min-confidence 60api_*Name(Load)/Attribute/ImportFrom)NAMEtokens only, docstrings/comments excludedadd_*graph overroutes/+server.py; 637 literal paths, 813 handler names)*.py *.ts *.tsx *.js *.md *.json *.yml *.yaml *.toml *.sh *.ps1 *.cfg *.txt) bucketed self/scope/prod/tests/docs/config/webwebsite/,mcp*,apps/,skills/,cli*,cron*,docs/)Pass D's four apparent orphans —
api_session_control_create/_stop/_send/_read— are all string-dispatch registered viaserver.py:1286_deferred_session_control("api_session_control_create")and are alive. A name-based scanner cannot see any of them. A module-level pass also confirmed every one of the 74 modules in the group has external references, so there are no dead modules.Deferred — 30-day gate
_RELEASE_CHANNELS(handlers/updates.py:152) — the only symbol in the group with zero real NAME tokens repo-wide, and a byte-identical duplicate of the live validatorplatform/update_layout.RELEASE_CHANNELS:25(which does the real work at lines 124 and 147, so validation is not missing). Refactor residue from feat(update): one derivation of install shape to update capability #2856. Held becausegit log -Sdates its introduction to 2026-08-05 and its last occurrence change to 2026-08-20 — both inside 30 days.Deferred — test-only, removal would be a refactor
Zero production callers, N test call sites; deleting the symbol means rewriting every fixture that uses it as a state seed or a differential oracle.
reset_link_meta_cache(link_meta.py:519)_reset_hook_inflight(hooks.py:403)_resolve_loaded_by_agents(_shared.py:828)s["loaded_by_agents"] == _resolve_loaded_by_agents(p)against the bulk production path_list_completions(terminal.py:1272)_resolve_completion_dir+_vetted_completion_dir+_list_vetted_completionsdirectlyREGISTRAR_NAMES(routes/__init__.py:58)test_dashboard_route_table.pypins order againstReported for a maintainer decision, not deleted
Half-wired lifecycle (the fix is wiring, not deletion):
_cached_parse_sessions(usage.py:1798) — the module comment at line 56 says this cache is "used byapi_usage". It is not: no production caller exists, andusage.py:1859re-implements the off-loop call as a barerun_in_executor(None, _parse_sessions)without the 120s TTL cache or the_SESSIONS_CACHE_LOCKcold-cache collapse, so a concurrent burst re-runs the fulliterdir+ per-filestat+ line-by-line parse every time._RingLogHandler._max(updates.py:1828) — written, never read; the only cap that exists is the deque's ownmaxlen, so_RingLogHandler(ring, max_size)silently ignoresmax_size.test_dashboard_updates_coverage.py:892passes it, so removing the parameter is a test-touching change.Registered but unreachable endpoint:
GET /api/sso-ttl—handlers_system.py:750api_sso_ttl, registeredroutes/realtime.py:55, re-exportedhandlers/__init__.py:19. The literal path appears nowhere else: not inwebsite/,test/,docs/, nor any MCP tool, app, skill, cron or CLI path. Introduced by the de-Amazoning scrub chore: scrub remaining Amazon-internal markers from public fork #168 — in the Amazon edition the companion adapter returned a real SSO TTL; in the OSS buildcurrent_context().identity.statushits the Default adapter, which spawns up to 4 subprocesses at a 5s timeout each. An authenticated GET with real cost and no caller. Held because it is registered, so it fails condition (1) of the death test, and removing an endpoint is a product decision that would also move the(method, path, handler)sequence pinned bytest_dashboard_route_table.py.Security surface — reported, not touched:
count_effective_denied_commands(security.py:338) — zero production callers; onlytest_denied_commands_api.py:158asserts on it. It computes the effective denied-command count and nothing surfaces it; the deadness may itself be the gap._load_bearer_token(kiro_usage_api.py:492) — zero production callers, 10 test sites, but superseded by design: its own docstring says it "applies NO ownership proof, so it must not be used to choose the credential a request is made with", andfetch_usage_limitsuses_candidate_tokens()directly.Out of scope, flagged for whoever audits
handlers_project.py:routes/connections.py:62-65registersGET /api/activities,POST /api/comments,GET /api/commentsandDELETE /api/comments/{id}, whose paths appear nowhere outside the registration andtest_project_alias.py. The handlers live indashboard/handlers_project.py, outside this group.Verification
mypy src/kiro_crew/— no issues in 1170 source files.test_artifacts_handlers.pyreproduce identically on unmodifiedorigin/main(artifacts.pyis not in this diff) and are left alone.gpt-5.6-sol,claude-opus-4.8) each returned No findings after independently checking dynamic dispatch, guard-behaviour equivalence,cdn_basespurity, the F401 risk, and doc/spec staleness.