ci: Harden the CI Gate and Clear the Zero-Risk Backlog

[droid-ash]

Meta

Change ID	Type	Confidence	Plan	Review
uloy	ci	4.9/5.0	11/11 tasks, 17/17 acceptance ✓	✓ 1 cycle

Impact	+/−	Net
true	+616 / −50	+566
└ impl	+559 / −50	+509
└ tests	+57 / −0	+57

_{generated by fab-kit v2.16.8}

Pipeline: intake ✓ → apply ✓ → review ✓ → hydrate ✓ → ship → review-pr

Summary

Thirteenth change in the code-quality initiative, and the first aimed at the initiative's own infrastructure rather than at warnings.

Two holes in the gate, both closed.

1. The gate never typechecked. #162 established this empirically: a type error injected into a report-web test fixture passed npm run build, npm test and npm run lint — only tsc -p caught it, and nothing invoked that. The gate now runs npm ci → build → typecheck → test → lint. Six per-package "typecheck": "tsc --noEmit -p tsconfig.json" scripts, fanned out from a root script deliberately without --if-present, so a missing script or tsconfig fails loudly (npm Missing script, or TS5058).

All six packages typecheck all their code including tests — newly for report-web, whose tsup/vite build never runs tsc, and redundantly for the five tsc-building packages. The redundancy is the point: it makes coverage explicit rather than a build-tool by-product that a future switch could silently remove, exactly as happened to report-web.

2. --if-present made a lost script indistinguishable from an intentional one. It is now gone from the gate's build and test steps and from the root scripts. packages/local-runtime (tarball packaging, no src/) declares explicit no-op build/test/typecheck scripts that state why and exit 0. The principle: a fan-out that skips on absence cannot express intent, so the exemption is declared in the package rather than inferred from what's missing. No blanket || true anywhere.

Both fixes are proven bidirectionally, and independently re-verified by the orchestrator and again by review:

• Inject a type error → npm run typecheck exit 2 (TS2322/TS2353); revert → exit 0.
• Delete a workspace's test script → npm run test:workspaces exit 1 with Missing script: "test" naming the workspace; restore → exit 0.

Zero-risk backlog cleared alongside. Four provably-dead constructs removed — each found by a review mutation surviving, and each re-verified unreachable before deletion rather than taken on trust:

• ActionExecutor's _runSingleDeviceAction failureMessage parameter plus its ten per-action fallback strings (_executeDeviceAction already substitutes 'Action failed', so result.error is never nullish).
• AIAgent.ts's throw lastError ?? new Error(exhaustedMessage) — simplified, not deleted; TypeScript still needs the terminal throw, and the new message is byte-identical to the removed literals. Dropping lastError was also required to keep no-unused-vars at zero.
• viewModel.ts's Math.max(0, …) clamp in formatRelativeTime (only that site — the clamp in formatVideoTimestamp and several in runDetailController.ts are live and untouched).
• logs.ts's if (!logText) return []; guard.

Plus five test assertions the previous reviews found missing, each mutation-verified: the unlink-order pin in cloud-core (spec zip before app zip — guaranteed by JS finally semantics and confirmed empirically in #157, but every existing assertion compared final sets, never sequence), and four boundary values in report-web (formatLongDuration round-up, formatVideoTimestamp truncation, the 24h→day boundary, resolveStepReasoning's think-before-plan precedence).

Scope was narrowed by agreement. A triage found 16 open deferred items, not the ten previously reported — and three listed as open were already done (#157's app-zip leak, #158's collision, artifacts.ts's half-pin inside #162). The queue spans infra, deletions, additive assertions, six behaviour-changing error-path fixes, and four separate decisions. This change takes only the infra and the provably non-behavioural items, so one review certifies the whole thing.

Verification: npm run build --workspaces exit 0 (note: no longer --if-present) · npm run typecheck exit 0, all seven workspaces · npm run test:workspaces exit 0, 460 tests / 0 fail (75 common, 19 cloud-core, 91 device-node, 67 goal-executor, 58 report-web, 150 cli) · npm run lint exit 0, 78 warnings / 0 errors unchanged, max-depth and no-unused-vars still zero.

Review verification

Review independently re-derived every dead-code claim rather than trusting the prior findings — confirming _executeDeviceAction is the sole producer and result.error non-nullish for every reachable value (including the '' case, where old and new are byte-identical), that AIAgent's loop cannot fall through for MAX_LLM_ATTEMPTS = 2 and the real error still reaches callers, that a negative delta and the NaN path both produce identical output without the clamp, and that only the intended Math.max site changed. It also verified build-before-typecheck is genuinely required (moving packages/common/dist aside makes dependents fail TS2307), ruled out an incremental-build false green by injecting an error after a build, confirmed release.yml is unaffected (it names workspaces explicitly and never uses --workspaces), and confirmed the fs.unlinkSync spy really observes order — reordering the releases fails the new pin while the set-based test stays green.

Known / follow-ups

1. A caveat on one removal, recorded rather than glossed. parseDeviceLogLines is re-exported through the published @finalrun/report-web/ui barrel. In-repo the guard removal is provably safe (the sole caller coerces with ?? '', and logText is typed string), so it is not a behaviour change under the declared contract — but an untyped external consumer passing undefined now gets a TypeError where it previously got []. That is a semantic change at a package boundary. Now recorded in report-web memory as a Design Decision: unreachable-under-the-types is only as strong as the declared types, and a published barrel widens who the callers are.
2. Still queued: the six error-path behaviour fixes (_trimmed guard, emulator output cap, getPlatform() swap, adbPath! guard, timeout message/parser mismatch, acquisition-side orphan); Dependabot's 4 frozen CVEs; a DOM test environment (would unlock report-web's 14 warnings); GrounderResponseConverter characterization.
3. docs/memory/ci/pr-quality-gate.md is now ~38KB against a ~15KB soft cap. The ci domain split is a queued /docs-reorg-memory job and is overdue.

Summary by CodeRabbit

• New Features

  • Added workspace-wide type checking to the build and validation workflow.
  • Added explicit package validation scripts, including intentional no-op checks where applicable.

• Bug Fixes

  • Improved handling of empty device logs and future timestamps.
  • Standardized retry and device-action error reporting.

• Tests

  • Added coverage for cleanup ordering, formatting boundaries, timestamp behavior, reasoning precedence, and video timestamps.

• Documentation

  • Updated CI quality-gate and reporting documentation to reflect the revised validation pipeline.

[coderabbitai]

Warning

Review limit reached

@droid-ash, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 48 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d50c7538-e0cd-4380-974f-fdbf3068a979

📥 Commits

Reviewing files that changed from the base of the PR and between 7f2ef07 and cae5966.

📒 Files selected for processing (3)

• fab/changes/260728-uloy-harden-gate-clear-safe-backlog/.history.jsonl
• fab/changes/260728-uloy-harden-gate-clear-safe-backlog/.status.yaml
• fab/changes/260728-uloy-harden-gate-clear-safe-backlog/intake.md

📝 Walkthrough

Walkthrough

The PR hardens the CI gate with explicit workspace typechecking and strict script discovery, simplifies selected executor and report-web logic, adds regression assertions, and updates CI, report-web, and change-history documentation.

Changes

CI gate hardening

Layer / File(s)	Summary
Workspace gate wiring .github/workflows/ci.yml, package.json, packages/*/package.json	CI now runs build, typecheck, test, and lint stages; workspace scripts no longer silently skip missing commands, and local-runtime declares explicit no-op scripts.
CI contract documentation docs/memory/ci/*, docs/memory/index.md	Documentation records the strict gate order, per-workspace tsc --noEmit checks, failure behavior, and lint’s non-blocking status.
Gate hardening change record fab/changes/260728-uloy-harden-gate-clear-safe-backlog/*	Intake, planning, status, and history records describe the scope, validation pins, acceptance criteria, and execution stages.

Behavior cleanup and regression coverage

Layer / File(s)	Summary
Executor error-path simplification packages/goal-executor/src/ActionExecutor.ts, packages/goal-executor/src/ai/AIAgent.ts	Device actions use driver errors directly, while LLM retry exhaustion uses a standardized generic error after preserving retries and fatal-error propagation.
Cleanup and boundary regression pins packages/cloud-core/src/test/submit.test.ts, packages/report-web/src/ui/*	Tests pin temporary-zip unlink order and report-web duration, reasoning, relative-time, and timestamp boundaries; parsing and relative-time guards are simplified.
Report-web contract documentation docs/memory/report-web/*	Report-web documentation records manifest/log-loading locations and the declared input-contract decision for UI barrel functions.

Archived report-web testing record

Layer / File(s)	Summary
Archived intake and scope fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/intake.md	The intake records characterization-test scope, strict runner requirements, exclusions, documentation targets, and assumptions.
Archived characterization plan fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/plan.md	The plan defines module pins, runner behavior, mutation verification, acceptance criteria, and execution tasks.
Archived execution records fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/.history.jsonl, .../.status.yaml, fab/changes/archive/index.md	Archive metadata records workflow progress, confidence, impact metrics, final status, and the archive index entry.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CIWorkflow
  participant WorkspaceScripts
  participant TypeScript
  participant WorkspaceTests
  CIWorkflow->>WorkspaceScripts: Run workspace build
  CIWorkflow->>WorkspaceScripts: Run workspace typecheck
  WorkspaceScripts->>TypeScript: Execute tsc --noEmit -p tsconfig.json
  CIWorkflow->>WorkspaceTests: Run workspace tests
  CIWorkflow->>WorkspaceTests: Run lint as non-blocking phase

Loading

Possibly related PRs

• droid-ash/finalrun-agent#151: Earlier CI gate setup and step ordering are extended by this PR.
• droid-ash/finalrun-agent#159: Related goal-executor internal error-handling refactoring.
• droid-ash/finalrun-agent#162: Related report-web characterization tests and strict test-runner behavior.

Suggested labels: enhancement

Suggested reviewers: arnoldlaishram

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: CI gate hardening plus cleanup of the zero-risk backlog.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 260728-uloy-harden-gate-clear-safe-backlog

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@fab/changes/260728-uloy-harden-gate-clear-safe-backlog/intake.md`:
- Line 129: Update the issue reference in the affected sentence so it does not
begin with “#162.” and trigger Markdown heading parsing; keep the reference
inline, such as “issue `#162`,” while preserving the surrounding text.

In
`@fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/.status.yaml`:
- Around line 19-25: Reconcile the confidence metrics using the authoritative
assumption classification in intake.md lines 163-178. Update .status.yaml lines
19-25 so certain and confident match those counts, documenting the derivation if
needed; then update plan.md lines 311-323 so its table and closing summary match
intake.md and .status.yaml. No direct change is required outside these three
records.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3272e5cb-0fa6-4083-b5fd-d9f5d2f55ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 0f2126a and 7f2ef07.

📒 Files selected for processing (30)

• .github/workflows/ci.yml
• docs/memory/ci/index.md
• docs/memory/ci/pr-quality-gate.md
• docs/memory/index.md
• docs/memory/report-web/index.md
• docs/memory/report-web/renderers.md
• fab/changes/260728-uloy-harden-gate-clear-safe-backlog/.history.jsonl
• fab/changes/260728-uloy-harden-gate-clear-safe-backlog/.status.yaml
• fab/changes/260728-uloy-harden-gate-clear-safe-backlog/intake.md
• fab/changes/260728-uloy-harden-gate-clear-safe-backlog/plan.md
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/.history.jsonl
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/.status.yaml
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/intake.md
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/plan.md
• fab/changes/archive/index.md
• package.json
• packages/cli/package.json
• packages/cloud-core/package.json
• packages/cloud-core/src/test/submit.test.ts
• packages/common/package.json
• packages/device-node/package.json
• packages/goal-executor/package.json
• packages/goal-executor/src/ActionExecutor.ts
• packages/goal-executor/src/ai/AIAgent.ts
• packages/local-runtime/package.json
• packages/report-web/package.json
• packages/report-web/src/ui/logs.ts
• packages/report-web/src/ui/test/format.test.ts
• packages/report-web/src/ui/test/viewModel.test.ts
• packages/report-web/src/ui/viewModel.ts

[coderabbitai]

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@fab/changes/260728-uloy-harden-gate-clear-safe-backlog/intake.md`:
- Line 129: Update the issue reference in the affected sentence so it does not
begin with “#162.” and trigger Markdown heading parsing; keep the reference
inline, such as “issue `#162`,” while preserving the surrounding text.

In
`@fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/.status.yaml`:
- Around line 19-25: Reconcile the confidence metrics using the authoritative
assumption classification in intake.md lines 163-178. Update .status.yaml lines
19-25 so certain and confident match those counts, documenting the derivation if
needed; then update plan.md lines 311-323 so its table and closing summary match
intake.md and .status.yaml. No direct change is required outside these three
records.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3272e5cb-0fa6-4083-b5fd-d9f5d2f55ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 0f2126a and 7f2ef07.

📒 Files selected for processing (30)

• .github/workflows/ci.yml
• docs/memory/ci/index.md
• docs/memory/ci/pr-quality-gate.md
• docs/memory/index.md
• docs/memory/report-web/index.md
• docs/memory/report-web/renderers.md
• fab/changes/260728-uloy-harden-gate-clear-safe-backlog/.history.jsonl
• fab/changes/260728-uloy-harden-gate-clear-safe-backlog/.status.yaml
• fab/changes/260728-uloy-harden-gate-clear-safe-backlog/intake.md
• fab/changes/260728-uloy-harden-gate-clear-safe-backlog/plan.md
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/.history.jsonl
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/.status.yaml
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/intake.md
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/plan.md
• fab/changes/archive/index.md
• package.json
• packages/cli/package.json
• packages/cloud-core/package.json
• packages/cloud-core/src/test/submit.test.ts
• packages/common/package.json
• packages/device-node/package.json
• packages/goal-executor/package.json
• packages/goal-executor/src/ActionExecutor.ts
• packages/goal-executor/src/ai/AIAgent.ts
• packages/local-runtime/package.json
• packages/report-web/package.json
• packages/report-web/src/ui/logs.ts
• packages/report-web/src/ui/test/format.test.ts
• packages/report-web/src/ui/test/viewModel.test.ts
• packages/report-web/src/ui/viewModel.ts

🛑 Comments failed to post (1)

fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/.status.yaml (1)

19-25: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Reconcile the archived confidence metrics across all change records.

The confidence classifications are inconsistent across the generated status and its source documents.

• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/.status.yaml#L19-L25: align certain/confident with the authoritative assumption counts or document their derivation.
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/intake.md#L163-L178: preserve the authoritative assumption classification used for aggregation.
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/plan.md#L311-L323: reconcile the table and closing summary with the intake and status records.

📍 Affects 3 files

• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/.status.yaml#L19-L25 (this comment)
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/intake.md#L163-L178
• fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/plan.md#L311-L323

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@fab/changes/archive/2026/07/260727-e5nk-backfill-report-web-logic-tests/.status.yaml`
around lines 19 - 25, Reconcile the confidence metrics using the authoritative
assumption classification in intake.md lines 163-178. Update .status.yaml lines
19-25 so certain and confident match those counts, documenting the derivation if
needed; then update plan.md lines 311-323 so its table and closing summary match
intake.md and .status.yaml. No direct change is required outside these three
records.

[droid-ash]

Review dispositions — 1 fixed, 1 diagnosed backwards (45ef210)

Fixed — line-leading #162.

Rewrapped as PR #162. (For the record: CommonMark requires a space after the # sequence, so #162. was never parsed as a heading — but no line should open with a bare issue number, so the change stands regardless.)

The .status.yaml finding — right that something is wrong, wrong about which file

You asked that the archived e5nk .status.yaml confidence metrics be edited to match intake.md's table. That's the wrong direction, and editing it would falsify computed output.

.status.yaml is generated by fab score, which derives each grade from its composite (0.2S + 0.3R + 0.3A + 0.2D). Per the project's own SRAD framework, the written Grade column is a reader hint that "can never contradict its own dimensions" — so where the two disagree, the computed value is authoritative and the hand-written label is the error.

I recomputed e5nk's composites directly:

9 rows >= 80  -> Certain
1 row  = 79.5 -> Confident

That is exactly the 9/1 .status.yaml already records. It was never wrong.

But the finding surfaced a real defect, and likely a systemic one — I've been hand-writing Grade labels that can disagree with their own S/R/A/D numbers. Rather than retro-edit an archived, merged change, I checked this change for the same fault and found it: row 2 written Confident, dimensions computing to 82 → Certain. Corrected the row and the footer; fab score now agrees at 8 certain / 2 confident.

So: the authoritative record stays untouched, the derived label that was actually wrong is fixed, and the class of error is now known rather than latent. Thanks — that one was worth chasing.

Verification: typecheck exit 0 · test:workspaces exit 0, 460 tests / 0 fail · lint exit 0, 78 warnings / 0 errors.