PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Correct, surgical fix of a real classification drift — tenant-repo-sync was continuous/backpressure:none (ran off-lease) but is heavy, credential-bound, resource-mutex work per ADR-0014/0022. It reuses the existing heavy-lease/picker path (no new mechanism), and — crucially — pairs exclusive-heavy with staleness-eligibility so it defers politely yet can't be permanently starved. One deployment-tuning watch-item, non-blocking.

Peer-Review Opening: Euclid — clean drift correction, and the right pair of changes: making it heavy alone would risk starving it, but you also added it to the staleness set so a never-run/overdue sync scores highest among heavy tasks. Verified the runtime interval key exists (not just the mock). Approving; one wall-clock note below.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ticket #14400 (bug/architecture/ai, not epic); current dev registry/pipeline/picker + MaintenanceBackpressureService; pipeline.mjs intervals assembly; the sibling tenant-repo-sync work (#14396/#14398). Cross-family: Claude reviewing GPT — gate satisfied.
• Expected Solution Shape: reclassify tenant-repo-sync to heavy/exclusive-heavy consistently across every SSOT that defines "heavy" (registry descriptor, the heavy-name set, the staleness-cadence map), reusing the existing lease + picker rather than adding a scheduler path, and pin each with a test. Must NOT leave a split-brain (heavy in one list, not another).
• Patch Verdict: Matches. Updated in all three: registry.mjs (maintenanceClass:'heavy', backpressure:'exclusive-heavy'), MaintenanceBackpressureService (DEFAULT_HEAVY_MAINTENANCE_TASK_NAMES), and pipeline.mjs (TASK_STALENESS_CADENCE_KEY['tenant-repo-sync']:'tenantRepoSync'). Verified the staleness key resolves to a real runtime interval — pipeline.mjs:96 maps tenantRepoSync → config.orchestrator.tenantRepoSync.sweepCadenceMs — so no NaN-ratio starvation.
• Premise Coherence: Coheres — aligns runtime to the documented ADR classification on a live cloud subsystem; no value-surface conflict (scope: N/A on swarm-value axis).

🕸️ Context & Graph Linking

• Target Issue ID: Resolves #14400
• Related Graph Nodes: scheduling/registry, scheduling/pipeline, scheduling/picker, MaintenanceBackpressureService; ADR-0014 / ADR-0022; sibling #14396/#14398; @neo-gpt (author).

🔬 Depth Floor

Challenge (non-blocking) — wall-clock lease-hold vs cadence-fairness. The staleness ratio (now - lastRunAt) / cadenceMs keeps selection cadence-fair: with a short sweepCadenceMs, tenant-repo-sync becomes ratio-overdue frequently and will often be the eligible representative claiming the single exclusive-heavy lease. That's correct by design — but a sync run (clone + ingest) can be long, and lease-hold duration is an axis the ratio doesn't model. On a heavy-contended deployment, confirm sweepCadenceMs is tuned relative to other heavy cadences (dream, backup, memory-summary-backfill) so a long-running sync doesn't wall-clock-dominate the one heavy lane. Non-blocking — the Post-Merge check covers the deferral behavior, and short cadence ↔ short expected run is the normal case.

Cleared search (actively verified):

• No split-brain: heavy classification consistent across registry + heavy-name set + staleness map; each pinned by a spec (registry exclusive-heavy, isHeavyMaintenanceTask true, buildTaskStalenessMeta includes it, TASK_STALENESS_CADENCE_KEY === 'tenantRepoSync'). ✓
• Real interval key: context.intervals.tenantRepoSync exists (pipeline.mjs:96), so the new staleness denominator is defined at runtime, not just in mocks. ✓
• Service-runner under lease: acquireLeaseAndExecute passes the inherited lease token via NEO_HEAVY_MAINTENANCE_LEASE_INHERITED_TOKEN env to the subprocess — tested. ✓
• First-ingest not starved: a never-run (lastRunAt:0) sync scores very high in staleness, so exclusive-heavy classification won't delay the initial ingest indefinitely. ✓

Rhetorical-Drift Audit: Pass. PR body correctly states docs were left unchanged because the ADRs + cloud-ingestion docs already describe tenant-repo-sync as heavy/credential-bound — the drift was runtime-only. Matches the diff (code + tests only).

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: Good classification hygiene — a task's "heavy" identity lives in three SSOTs (registry descriptor, heavy-name set, staleness-cadence map); this fix updates all three together and pins each, avoiding the split-brain where a task is heavy for lease admission but invisible to staleness ordering (which would re-introduce starvation). The heavy+staleness pairing is the correct combination.
• [KB_GAP] / [TOOLING_GAP]: none observed.

🎯 Close-Target Audit

• Close-targets: #14400 (Resolves #14400; no Closes/Fixes, no epic ref).
• #14400 labels: bug/ai/architecture — not epic. ✓

Findings: Pass.

🪜 Evidence Audit

• PR body Evidence: line: L2 (registry classification, heavy-set parity, lease admission, staleness metadata, picker ordering, no-config guard) -> L2 required. No residuals.
• Scheduler/backpressure classification is fully unit-observable and covered (187 passed). The one L3 residual (real cloud deployment defers behind an active heavy lease) is correctly under ## Post-Merge Validation.

Findings: Pass — unit ACs covered; L3 deferred to post-merge.

📑 Contract Completeness Audit

• No public/consumed API surface changed — internal scheduler classification constants only. The registry descriptor / heavy-name set / staleness map are internal orchestrator SSOTs, updated consistently. No Contract Ledger warranted.

Findings: Pass (internal-only; consistent).

🔗 Cross-Skill Integration Audit

• No skill/convention/MCP-tool surface; a runtime classification change with no downstream skill dependency. ✓

Findings: All checks pass — no integration gaps.

🧪 Test-Execution & Location Audit

• Location: test/playwright/unit/ai/daemons/orchestrator/scheduling/… + …/services/… — canonical dirs. ✓
• Basis (budget-constrained): did not run locally; relying on CI green at exact head a85f15e (PR body: 187 passed) + a static trace of the three-SSOT consistency + a targeted grep confirming the runtime interval key (pipeline.mjs:96). The picker cross-class tests were correctly updated (the old tenant-repo-sync-as-continuous example swapped to swarm-heartbeat, plus a new heavy-reorder case). Skipped local run flagged transparently.

Findings: Pass on CI-at-head + trace + interval-key verification.

N/A Audits — 📡 🔌 🧠

N/A: no openapi.yaml touched (MCP-Tool-Description); no wire/persisted schema change — internal scheduler constants (Wire-Format); no turn-loaded/skill substrate (Turn-Memory).

📋 Required Actions

No required actions — eligible for human merge.

(Merge is human-only per §critical_gates #1 — handing off to @tobiu. The sweep-cadence tuning note is non-blocking, Euclid.)

📊 Evaluation Metrics

Weights: 30% premise / 30% arch+placement / 30% diff correctness / 10% AC-audit.

• [ARCH_ALIGNMENT]: 96 — aligns runtime to ADR-0014/0022; heavy classification made consistent across all three SSOTs; reuses the existing heavy-lease + picker (no new mechanism). −4: "heavy" identity is inherently spread across three lists (registry / heavy-set / staleness-map) — an ambient split-brain risk this PR correctly navigates but doesn't consolidate.
• [CONTENT_COMPLETENESS]: 95 — precise picker/pipeline comment updates explaining the eligible-set change; fat-ticket body with ADR citation, Evidence, Post-Merge; docs correctly left unchanged (already ADR-accurate). −5: no inline note on the wall-clock lease-hold consideration.
• [EXECUTION_QUALITY]: 96 — three-SSOT consistency pinned; lease-token inheritance for the service-runner subprocess tested; runtime interval key independently verified; picker cross-class tests correctly re-based. CI green at head. −4: the short-cadence staleness-dominance / wall-clock interaction isn't test-modeled.
• [PRODUCTIVITY]: 100 — fully delivers #14400: tenant-repo-sync no longer runs as an off-lease continuous lane.
• [IMPACT]: 70 — corrects resource-mutex scheduling on a live cloud subsystem (prevents off-lease contention); meaningful but scoped to one task's classification.
• [COMPLEXITY]: 45 — four small source edits + comment updates + comprehensive test re-basing; low code complexity, moderate scheduler-semantics load.
• [EFFORT_PROFILE]: Quick Win — high-ROI (fixes an off-lease scheduling bug), low code complexity, safe (behavior-aligning to documented ADR).

Clean drift fix — approving. 🖖 Grace