Skip to content

chore(dashboard): remove confirmed dead code in the dashboard handlers - #6941

Merged
bolichen97 merged 1 commit into
mainfrom
chore/dead-code-dash-handlers
Aug 30, 2026
Merged

chore(dashboard): remove confirmed dead code in the dashboard handlers#6941
bolichen97 merged 1 commit into
mainfrom
chore/dead-code-dash-handlers

Conversation

@iamwhatever

Copy link
Copy Markdown
Collaborator

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_agenthandlers/agents.py:153-159 + re-export handlers/__init__.py:58

Exactly one real NAME token repo-wide, and it is the re-export line. handlers/__init__.py has no __all__ — it is a # noqa: F401 shim — so a name listed there with no importer is unreachable. Zero string refs, zero non-Python refs, zero getattr/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_agent keeps 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 = None stub — handlers/terminal.py:38, 43

The whole-word name occurs on exactly those two lines. The later hits (541, 918, 1586, 1602) are the English word "signal" in comments — tokenize classifies them COMMENT, not NAME. No signal. attribute access anywhere in the module, on either platform branch. Since the name is never Loaded, its absence raises no NameError and 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-used fcntl, _pty and termios.

3. The skipped tally in list_skill_treehandlers/_shared.py:1433, 1450, 1457, 1461

Initialised and incremented in three branches, never read: not returned, not logged, not in the response. AugAssign is a Store-only node, so the name is never Loaded.

The sensitive-path guard is untouched. Every is_sensitive_path(...) gate, the symlink-escape real.relative_to(skill_root...) check, the exception handler, and all four continue statements 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) logs count=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:2118

Inside api_update_channel; neither binding is read afterwards. cdn_bases() (platform/update_layout.py:167-175) reads KIROCREW_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 at updates.py:2309-2343update_blocked_reason(feed_base) or update_blocked_reason(artifact_base) plus _cdn_safe(). The sibling read at updates.py:1040 already used the feed_base, _artifact_base convention 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/*.py reaches it via app.router.add_get(path, handlers.api_x) and the request comes from website/src. So "has a caller" was replaced with "is registered", then with "is reached".

pass method raw kept
A vulture --min-confidence 60 190 ~150 were route-registered api_*
B AST (module defs minus repo-wide Name(Load)/Attribute/ImportFrom) 1 attribute-name collisions across 3065 files swamp it
C tokenize — real NAME tokens only, docstrings/comments excluded 1 the authoritative pass
D route reachability (add_* graph over routes/ + server.py; 637 literal paths, 813 handler names) 4 apparent orphans 0 real
E reverse index over 6987 files (*.py *.ts *.tsx *.js *.md *.json *.yml *.yaml *.toml *.sh *.ps1 *.cfg *.txt) bucketed self/scope/prod/tests/docs/config/web 1 zero-ref, 8 test-only
F AST unused-assigned-locals inside scope functions 2 both confirmed
G endpoint-path reachability (registered stem vs website/, mcp*, apps/, skills/, cli*, cron*, docs/) 6 1 in-scope, reported below

Pass D's four apparent orphans — api_session_control_create / _stop / _send / _read — are all string-dispatch registered via server.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 validator platform/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 because git log -S dates 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.

symbol tests shape
reset_link_meta_cache (link_meta.py:519) 3 test-support reset seam living in production code
_reset_hook_inflight (hooks.py:403) 4 same, across two test modules
_resolve_loaded_by_agents (_shared.py:828) 14 differential oracle — tests assert s["loaded_by_agents"] == _resolve_loaded_by_agents(p) against the bulk production path
_list_completions (terminal.py:1272) 18 superseded wrapper; the production path inlines _resolve_completion_dir + _vetted_completion_dir + _list_vetted_completions directly
REGISTRAR_NAMES (routes/__init__.py:58) 9 not dead — it is the route-table ratchet test_dashboard_route_table.py pins order against

Reported 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 by api_usage". It is not: no production caller exists, and usage.py:1859 re-implements the off-loop call as a bare run_in_executor(None, _parse_sessions) without the 120s TTL cache or the _SESSIONS_CACHE_LOCK cold-cache collapse, so a concurrent burst re-runs the full iterdir + per-file stat + line-by-line parse every time.
  • _RingLogHandler._max (updates.py:1828) — written, never read; the only cap that exists is the deque's own maxlen, so _RingLogHandler(ring, max_size) silently ignores max_size. test_dashboard_updates_coverage.py:892 passes it, so removing the parameter is a test-touching change.

Registered but unreachable endpoint:

  • GET /api/sso-ttlhandlers_system.py:750 api_sso_ttl, registered routes/realtime.py:55, re-exported handlers/__init__.py:19. The literal path appears nowhere else: not in website/, 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 build current_context().identity.status hits 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 by test_dashboard_route_table.py.

Security surface — reported, not touched:

  • count_effective_denied_commands (security.py:338) — zero production callers; only test_denied_commands_api.py:158 asserts 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", and fetch_usage_limits uses _candidate_tokens() directly.

Out of scope, flagged for whoever audits handlers_project.py: routes/connections.py:62-65 registers GET /api/activities, POST /api/comments, GET /api/comments and DELETE /api/comments/{id}, whose paths appear nowhere outside the registration and test_project_alias.py. The handlers live in dashboard/handlers_project.py, outside this group.

Verification

  • 17 deterministic backend gates green (black, isort, flake8, subprocess-encoding, lockdown-before-publish, brand-name, harness-parity, loop-bound-locks, testpaths-coverage, changelog-history, plus each one's self-test).
  • mypy src/kiro_crew/ — no issues in 1170 source files.
  • Affected-surface pytest (74 test files selected by the seams this diff touches): 3210 passed, 2 skipped. The 2 failures in test_artifacts_handlers.py reproduce identically on unmodified origin/main (artifacts.py is not in this diff) and are left alone.
  • Two local AI review lanes (gpt-5.6-sol, claude-opus-4.8) each returned No findings after independently checking dynamic dispatch, guard-behaviour equivalence, cdn_bases purity, the F401 risk, and doc/spec staleness.

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/.
@iamwhatever
iamwhatever requested a review from a team as a code owner August 30, 2026 06:00
@iamwhatever
iamwhatever requested a review from pepmach August 30, 2026 06:00
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Design-level review of 7baa995847e15b1a0b93b8378a1b56d420315aff — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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: _auto_install_agent has zero remaining references and its purpose is served by the live _commit_agent_config_write_installed_config_locked path (agents.py:289); signal in terminal.py survives only in comments; the skipped removal leaves every is_sensitive_path gate and continue intact; and cdn_bases() is pure (env read + return), with the safety gate living in the separate cdn_bases_are_safe(), so discarding the call changes nothing.

[DESIGN-REVIEWED] 7baa995

@github-actions

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 7baa995847e15b1a0b93b8378a1b56d420315aff — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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 (install_agent, _cdn_bases) keep counted live callers (4 in agents.py:821,850,900,910; 1 at updates.py:1040), cdn_bases() is a pure two-line env read so discarding its call is a no-op, and the sensitive-path guard in list_skill_tree keeps all its continue statements — only the never-read counter goes.

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 ships

Intent: delete confirmed-dead code from the dashboard handlers group — a FIX (removing unearned surface), not an addition.

  1. _auto_install_agent and its __init__.py re-export deleted — justified; zero references repo-wide at HEAD, install_agent import keeps 4 live callers.
  2. Dead import signal plus its Windows = None stub deleted — justified; remaining "signal" hits are comments only.
  3. Never-read skipped tally deleted from list_skill_tree — justified; all sensitive-path/symlink guards and continues intact.
  4. Discarded _, artifact_base = _cdn_bases() call deleted — justified; cdn_bases() (update_layout.py:167) is side-effect-free, sibling read at updates.py:1040 unaffected.

[FIRST-PRINCIPLES-REVIEWED] 7baa995

@github-actions

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 7baa995847e15b1a0b93b8378a1b56d420315aff — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 7baa995

Verdict parsed from the review's SHA-scoped output markers for commit 7baa995847e15b1a0b93b8378a1b56d420315aff.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 7baa995847e15b1a0b93b8378a1b56d420315aff: <one-sentence reason>

@github-actions

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 7baa995847e15b1a0b93b8378a1b56d420315aff and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 7baa995

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 7baa995847e15b1a0b93b8378a1b56d420315aff: <one-sentence reason>

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Aug 30, 2026

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

@bolichen97
bolichen97 enabled auto-merge (squash) August 30, 2026 08:32
@bolichen97
bolichen97 merged commit ba397d8 into main Aug 30, 2026
67 checks passed
@bolichen97
bolichen97 deleted the chore/dead-code-dash-handlers branch August 30, 2026 08:32
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 30, 2026
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