Skip to content

Epic: review-stack architecture audit — eliminate duplication, hardcoding, config sprawl, and dead code #4622

Description

@JSONbored

Context

This is the output of an extensive, codebase-only audit of the gittensory review engine
(src/**, packages/gittensory-engine/src/**, review-enrichment/src/**) requested to answer: where
is there hardcoded/legacy behavior specific to JSONbored's own 3 repos that should instead be generic and
self-hoster-configurable; where has duplication drifted into real bugs; where does the config surface
overlap or contradict itself; and where is dead code adding navigation cost. The audit is explicitly
framed around a future hosted/multi-tenant version — the goal is to reuse the existing review-engine
stack as much as possible for that eventuality, which means the config-resolution and per-feature
activation mechanisms need to be genuinely single-sourced, not just "mostly consistent."

This issue and its sub-issues are the deliverable of an audit-only pass — no code was changed while
producing them.
Five agents independently swept: duplication, hardcoding, decision-path/layering
(including a security-relevant bypass-path check), silent-drift (stale comments vs. actual behavior),
and dead code, plus a sixth pass specifically on config-schema sprawl. All findings below were verified
against the live codebase (grep, git-blame, cross-reference — not speculative) before being filed.

What the audit found, in one paragraph

The core review-decision path (planAgentMaintenanceActions) is genuinely well-unified — that's the
good news. But 5 hand-written "deterministic close" paths bypass it by design and have already drifted
into a live authorization gap (#4602). A confidence-floor config field has been silently inert since a
late-June refactor, with 5 stale comments still promising the old behavior (#4603). The secret-detection
logic has 3 duplicated copies that have drifted twice already, once causing false-positive auto-closes
(#4604, #4608 — this is the root cause class behind the false positives reported this week, now
independently reconfirmed by a fresh audit pass). processors.ts is 15,241 lines with a single
2,631-line function (#4607). At least 9 genuinely hardcoded, JSONbored-specific values wall off or
mis-brand a self-hoster's own deployment (#4612-#4615). Per-feature activation for ~16 advisory review
capabilities is implemented 4 different ways with 4 different config-precedence rules, which has already
caused one real production incident (#4616 — the single highest-impact finding in this audit).
RepositorySettings has grown to 102 fields with real redundancy (#4618). And 14+ exports plus 2 entire
orphaned DB tables are dead weight left over from features that were removed or superseded but not fully
cleaned up (#4619-#4621).

How to use this issue

Sub-issues are grouped by priority. P0 items are live correctness/security bugs discovered incidentally
during the audit
— they should generally be picked up before the broader architectural cleanup, since
they're real bugs regardless of the audit's original architecture-improvement framing. P1 items are the
core "simplify, consolidate, eliminate duplication/hardcoding/config-sprawl" work the audit was
requested to find. P2 is dead-code removal — safe, mechanical, do whenever convenient.

Every sub-issue is self-contained (file:line evidence, impact, fix shape, acceptance criteria) and can be
picked up independently. Several are natural batches (e.g. the hardcoding items) but were kept in
reasonably-scoped sub-issues rather than one giant catch-all PR, per this repo's own small-PR convention.

Safety note carried over from the audit instructions: none of this touches the live server or ORB
deployment directly — every sub-issue is a codebase-only change. Deploying any given fix still follows
the normal beta-image → edge-nl-01 rollout process, one fix at a time, same as always.

Sub-issues

P0 — live correctness/security bugs found during the audit

P1 — duplication / architecture consolidation

P1 — hardcoding / self-host genericity

P1 — config sprawl (the biggest lever for hosted-version reuse)

P1 — hardcoding follow-up

P2 — dead code cleanup (safe, mechanical)

Non-goals / explicitly out of scope for this epic

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.roadmapOn the Wave-2 agent-layer roadmap board (project 9)

Projects

Status
Todo

Relationships

None yet

Development

No branches or pull requests

Issue actions