Skip to content

chore: remove dead code, scratch artifacts, and harden deploy infra (#782) - #890

Merged
frankbria merged 1 commit into
mainfrom
fix/782-dead-code-infra-cleanup
Jul 24, 2026
Merged

chore: remove dead code, scratch artifacts, and harden deploy infra (#782)#890
frankbria merged 1 commit into
mainfrom
fix/782-dead-code-infra-cleanup

Conversation

@frankbria

Copy link
Copy Markdown
Owner

Closes #782

Summary

Batch cleanup from the SaaS launch-readiness audit (P3.11) — every flagged dead item is removed (none had product-code consumers), plus the two infra nits.

Dead code removed

  • MetricsTracker.get_token_usage_stats: always-empty by_day key, its docstring entry, and the TODO (real day aggregation lives in get_token_usage_timeseries)
  • codeframe/planning/issue_generator.py (IssueGenerator, parse_prd_features, assign_priority) + its orphaned test module; planning/__init__.py trimmed
  • conductor.py: two no-op expression statements (completed_count + failed_count + blocked_count) and the unread BatchRun.isolate field / start_batch(isolate=) param (superseded by isolation; never persisted — no DB column, so existing databases are unaffected)
  • claude-agent-sdk dependency (declared, never imported); uv.lock regenerated — exactly one package removed

Repo hygiene

  • Deleted tracked tests/*.py.backup (2) and tests/debug/ scratch tests (2)
  • .gitignore: generic *.backup and *.log rules added; stale tests/test_new_feature.py entry and now-redundant specific log entries dropped

Infra hardening

  • systemd/codeframe-health-check.timer: single OnCalendar=*-*-* 02:00:00 — multiple OnCalendar= lines are additive, so it was firing at both 00:00 and 02:00
  • .github/workflows/deploy.yml: top-level permissions: contents: read (verified sufficient for the reusable test.yml it calls); both deploy jobs now write a pinned SSH_KNOWN_HOSTS secret instead of live ssh-keyscan (TOFU), with a fail-fast guard when the secret is unset

⚠️ Action required before next deploy

Create the SSH_KNOWN_HOSTS repo secret or the next deploy fails fast at "Add server to known hosts". From a trusted session:

ssh-keyscan -H <staging-host>   # verify fingerprint out-of-band, then
gh secret set SSH_KNOWN_HOSTS   # paste the output (include entries for every deploy host)

Verification

  • ruff check . clean; strict mypy codeframe/ clean
  • Focused suites green (test_worktrees.py, test_conductor.py, tests/planning/); two timing-flaky conductor tests failed only under heavy parallel load and pass in isolation — unrelated to this diff (conductor changes are behavior-neutral removals)
  • Full non-e2e suite running locally; CI is the authoritative gate
  • Cross-family review (opencode/GLM): no Critical/Major findings; its one Minor (empty-secret guard) is included. CodeRabbit CLI: 0 findings.

Known limitations

  • get_token_usage_stats itself has no callers — out of scope here (issue names only by_day); candidate for a future cleanup issue
  • The SSH_KNOWN_HOSTS secret must be rotated manually if the VPS host key changes

…782)

- Remove no-op by_day field + TODO from MetricsTracker.get_token_usage_stats
- Delete orphaned IssueGenerator planning module and its tests
- Remove dead expressions and unread BatchRun.isolate field from conductor
- Drop unused claude-agent-sdk dependency (uv.lock regenerated)
- Delete tracked .py.backup and tests/debug scratch files
- .gitignore: add generic *.backup and *.log, drop stale/redundant entries
- systemd health-check timer: single 02:00 schedule (was firing at 00:00 too)
- deploy.yml: least-privilege permissions block; pinned SSH_KNOWN_HOSTS secret
  replaces live ssh-keyscan TOFU in both deploy jobs
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @frankbria's task in 4m 40s —— View job


GLM precision bug-hunting review

Scope: concrete defects only (logic errors, crashes, data loss, security, contract breaks, leaks). Style/coverage is out of scope — a general review handles those.

✅ GLM review: no defects found.

I verified each removal/behavior-change against the checked-out code, not just the diff:

Dead-code removals — confirmed zero product-code consumers:

  • MetricsTracker.get_token_usage_stats "by_day" key — repo-wide grep finds no caller reads stats["by_day"]; the only other by_day is an unrelated local var in token_repository.py:443.
  • IssueGenerator / parse_prd_features / assign_priority (and the planning/__init__.py re-exports) — no .py imports them; only stale docs reference the names.
  • BatchRun.isolate field + start_batch(isolate=) param — no caller passes isolate= or reads .isolate; not persisted (no DB column). The two removed completed_count + failed_count + blocked_count lines were bare no-op expressions; completed_count is still used right after (conductor.py:1652).
  • claude-agent-sdk dep — no import claude_agent_sdk anywhere in codeframe/; all matches are docs. uv.lock drops exactly that one package.

Infra changes — checked for real failure scenarios:

  • .github/workflows/deploy.yml permissions: contents: read — verified it does not downscope-break the reusable test.yml: test.yml declares only contents: read and uses no write-scope step (no gh, no check-run/PR-comment creation, no pages/packages/statuses/checks/pull-requests scopes; upload-artifact and action caching need no token write scope). The two deploy jobs themselves only checkout + SSH outward + write $GITHUB_STEP_SUMMARY (no scope needed). Sufficient.
  • SSH pinned-host change — printf '%s\n' "${SSH_KNOWN_HOSTS}" writes the secret verbatim with %s (no format/backslash interpretation); the -z guard fails fast on an unset secret. Sound.
  • systemd/…health-check.timer — multiple OnCalendar= are additive, so the old pair (daily→00:00 + 02:00→02:00) fired twice; the single *-*-* 02:00:00 correctly fires once at 02:00. Correct fix.
  • .gitignore — broadening specific *.log/server.log*.log and adding *.backup strictly widens coverage (no regression); the dropped tests/test_new_feature.py entry is harmless — that file neither exists nor is tracked.

No defect could be stated with a concrete failure scenario, so nothing is reported.

@frankbria

Copy link
Copy Markdown
Owner Author

Demo: acceptance criteria → outcome evidence

All commands run against the PR branch working tree.

1. Each dead item removed, packages still import

$ grep -rn "IssueGenerator|parse_prd_features|assign_priority|claude_agent_sdk" --include="*.py" codeframe/ tests/ | wc -l
0
$ grep -rn '"by_day"' codeframe/lib/metrics_tracker.py | wc -l
0
$ grep -c "isolate: bool|isolate=" codeframe/core/conductor.py
0
$ uv run python -c "import codeframe.planning, codeframe.core.conductor, codeframe.lib.metrics_tracker; print('imports OK')"
imports OK
$ grep -c claude-agent-sdk pyproject.toml uv.lock
pyproject.toml:0 / uv.lock:0

2. permissions: {contents: read} in deploy.yml

$ grep -n -A1 "^permissions:" .github/workflows/deploy.yml
20:permissions:
21-  contents: read

3. SSH_KNOWN_HOSTS pinned (no live keyscan)

$ grep -c 'ssh-keyscan -H ${{ secrets.HOST }}' .github/workflows/deploy.yml
0
$ grep -n 'printf' .github/workflows/deploy.yml | grep KNOWN_HOSTS
78:          printf '%s\n' "${SSH_KNOWN_HOSTS}" >> ~/.ssh/known_hosts    (deploy-staging)
330:          printf '%s\n' "${SSH_KNOWN_HOSTS}" >> ~/.ssh/known_hosts   (deploy-production)

Plus a fail-fast guard when the secret is unset (lines 73/325).

4. OnCalendar=daily dropped — timer fires once

$ grep OnCalendar systemd/codeframe-health-check.timer
OnCalendar=*-*-* 02:00:00
$ systemd-analyze calendar '*-*-* 02:00:00'
Normalized form: *-*-* 02:00:00
    Next elapse: Fri 2026-07-24 02:00:00

5. Backup/scratch/placeholder tests deleted

$ git ls-files | grep -cE "\.backup$|^tests/debug/"
0

6. .gitignore covers *.backup / *.log / coverage.json

$ git check-ignore -v foo.backup foo.log coverage.json
.gitignore:58:*.backup      foo.backup
.gitignore:45:*.log         foo.log
.gitignore:43:coverage.json coverage.json

Quality gates: ruff clean, strict mypy clean, CodeRabbit CLI 0 findings, opencode (GLM) cross-family review: no Critical/Major (its one Minor — empty-secret guard — is included in this PR).

@frankbria
frankbria merged commit 9de234c into main Jul 24, 2026
10 of 11 checks passed
@frankbria
frankbria deleted the fix/782-dead-code-infra-cleanup branch July 24, 2026 09:31
frankbria added a commit that referenced this pull request Jul 30, 2026
Deploy has failed on every main push since #890 (2026-07-24) because that
PR replaced live ssh-keyscan TOFU with a pinned host key from the
SSH_KNOWN_HOSTS secret — but the secret was never populated, so the
'Add server to known hosts' step exits 1 on both the staging and
production jobs.

The real fix is operational (set the secret from a trusted machine after
verifying the host fingerprint out-of-band). This change just makes the
guard emit the exact command instead of pointing at a comment, matching
the AUTH_SECRET guard already in this file.
frankbria added a commit that referenced this pull request Jul 30, 2026
Deploy has failed on every main push since #890 (2026-07-24) because that
PR replaced live ssh-keyscan TOFU with a pinned host key from the
SSH_KNOWN_HOSTS secret — but the secret was never populated, so the
'Add server to known hosts' step exits 1 on both the staging and
production jobs.

The real fix is operational (set the secret from a trusted machine after
verifying the host fingerprint out-of-band). This change just makes the
guard emit the exact command instead of pointing at a comment, matching
the AUTH_SECRET guard already in this file.
frankbria added a commit that referenced this pull request Jul 30, 2026
…ing, pm2 env) (#1002)

* fix(ci): make Deploy's missing SSH_KNOWN_HOSTS error actionable

Deploy has failed on every main push since #890 (2026-07-24) because that
PR replaced live ssh-keyscan TOFU with a pinned host key from the
SSH_KNOWN_HOSTS secret — but the secret was never populated, so the
'Add server to known hosts' step exits 1 on both the staging and
production jobs.

The real fix is operational (set the secret from a trusted machine after
verifying the host fingerprint out-of-band). This change just makes the
guard emit the exact command instead of pointing at a comment, matching
the AUTH_SECRET guard already in this file.

* ci(deploy): reach the staging/prod box over Tailscale, not public :22

The VPS firewalls SSH off the public internet, so GitHub runners time out
connecting to port 22 (they aren't on the box's allowlist). Add a Tailscale
connect step to both deploy jobs so the runner joins the tailnet and reaches
sshd privately — port 22 stays closed to the world.

Requires (documented in deploy/README.md): TS_OAUTH_CLIENT_ID/SECRET secrets,
the box on the tailnet with 'ufw allow in on tailscale0 to any port 22', a
tag:ci ACL, HOST repointed to the tailnet name/IP, and SSH_KNOWN_HOSTS
relabelled to that HOST.

* fix(deploy): cold-start pm2 from ecosystem so new .env vars propagate

The inline deploy script restarted the backend with `pm2 restart <name>
--update-env`, which refreshes env from the deploy shell — but the shell
never sources .env.staging, so vars ADDED to the env file (WORKSPACE_ROOT,
CODEFRAME_DEPLOYMENT_MODE from #896) never reached the process. Only a fresh
`pm2 start <ecosystem>` runs the ecosystem's dotenv.config() that loads the
file. With the process already online it took the restart branch every time,
so the backend crash-looped on startup ('WORKSPACE_ROOT must be set') and the
health check failed — the last hidden breakage behind the deploy failures.

Delete the codeframe apps by name (not `pm2 delete all`, which would kill
unrelated apps on this shared VPS) and start fresh from the ecosystem, which
re-reads .env every deploy. Same fix applied to the production job, which had
the more basic `pm2 restart <name>` with no env refresh at all.
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.

[P3.11] Dead/unused code and minor infra nits — batch cleanup

1 participant