Skip to content

fix(service-settings): wire the typecheck script and clear the 14 errors the unexecuted gate hid - #7959

Merged
huangyiirene merged 1 commit into
mainfrom
claude/issue-7925-service-settings-typecheck
Aug 12, 2026
Merged

fix(service-settings): wire the typecheck script and clear the 14 errors the unexecuted gate hid#7959
huangyiirene merged 1 commit into
mainfrom
claude/issue-7925-service-settings-typecheck

Conversation

@huangyiirene

@huangyiirene huangyiirene commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Fixes #7925

The premise, re-measured on this branch's base (6ff179d)

Confirmed, exactly as filed:

  • packages/services/service-settings/package.json declared only build and test.
  • turbo run typecheck --filter=@objectstack/service-settings exited 0 having run 6 tasks — all of them ^build dependencies, never a service-settings:typecheck. Turbo silently no-ops a package missing the script and reports success.
  • An ad-hoc tsc --noEmit in the package surfaced 14 errors across the five named test files. Note the package's tests live under src/, so the tsconfig include always reached them — only the script that runs the compiler was missing.

After the one-line script (mirroring sibling service-messaging), the same command runs 7 tasks, the new one being a real service-settings:typecheck, and it is green.

Per-error verdict — 14 of 14 are stale tests

No service-settings/src/** non-test file changed. Every error was a test that had drifted from a contract that legitimately grew.

File n Error Verdict
manifests/sms.manifest.test.ts 3 TS2345 — handler called { values, ctx } stale test
manifests/storage.manifest.test.ts 4 TS2345 — same stale test
manifests/ai.manifest.test.ts 1 TS6133 — aiTestEmbedderActionHandler imported, never used stale test
settings-service.test.ts 1 TS2322 — number not assignable to void | Promise<void> stale test
translations/settings-translation-coverage.test.ts 5 TS2677 + 4 cascading stale test

Details:

  • The action-handler calls (7). SettingsActionHandler takes { namespace, actionId, values, payload?, ctx }, and settings-service.ts:1809 — the only invocation site — passes all of it. The tests still used the older two-field shape. They now call handlers the way the service does (namespace: 'sms' | 'storage', actionId: 'test', taken from each manifest's own action_button id). SettingsContext is all-optional, so ctx: {} type-checks with no cast.
  • ai.manifest.test.ts. Its five calls were already as any — the same drift, pre-silenced. Those casts are removed, not extended to the other two files. The unused import was there because aiTestEmbedderActionHandler was never tested despite the manifest declaring a test_embedder button for it; rather than delete the import, its two branches (disabled provider, missing API key) are now covered. Net +2 tests.
  • settings-service.test.ts:143. record: (e) => events.push(e) returned Array.push's number from a sink declared Promise<void> | void. Braced.
  • settings-translation-coverage.test.ts. It filtered the manifests barrel through a hand-rolled structural Manifest/Specifier pair that had drifted from the real shape (label is string | Record<string, string>, not string), so its type predicate was not assignable to the union it narrowed — TS2677, with four cascading errors downstream because an invalid predicate leaves filter returning the unnarrowed union. It now narrows to the spec's own SettingsManifest; the local duplicates are deleted.

Nothing was silenced. No any added, no @ts-expect-error, no change to the tsconfig include, and five existing as any casts removed.

The sweep — and a correction to the card's framing

The card suggests sweeping for other packages missing the script, on the premise that the silent no-op is untracked everywhere. That premise is false, and worth stating plainly: the invariant is already gated. scripts/check-type-check-coverage.mjs (check:type-check-coverage / check:type-check-debt, #4311) already enforces exactly this — every workspace package either declares a typecheck script or carries a measured DEBT entry with an error count and a tracking issue, and the ledger is closed to new debt.

service-settings was in that ledger at errors: 13, with a note naming the very errors this PR fixes ("TS2345 x7: manifest action handlers called without namespace/actionId; TS2322"). So this was tracked debt being paid down, not an unknown hole.

The sweep (jq over all 78 workspace package.jsons) finds 13 remaining packages without the script, and they reconcile exactly with the ledger — 13 DEBT entries + 1 EXEMPT (@objectstack/console, a published artifact with no TS sources), where one DEBT entry is the workspace root rather than a package directory:

Package Ledgered raw errors
@objectstack/core 98
@objectstack/metadata 92
@objectstack/spec-monorepo (workspace root) 80
@objectstack/metadata-protocol 63
@objectstack/service-storage 52
@objectstack/cloud-connection 13
@objectstack/observability 11
@objectstack/service-knowledge 10
@objectstack/service-analytics 10
@objectstack/service-automation 5
@objectstack/knowledge-ragflow 4
@objectstack/hono 3
@objectstack/service-cluster 1
@objectstack/console EXEMPT
total 442

Scoping decision: wire none of them here. Triage on #7925 already fenced the sweep out of this card (routing the mechanical invariant to #7849), and the measurement agrees — these are not a handful of clean packages, they are 442 frozen raw errors of already-tracked debt, four of them over 50. Wiring any of them means paying its burn-down, since this PR must leave green everything it wires. The survey is the deliverable; the burn-down belongs to #4311's per-package cards.

Ratchets — moved down, none raised

Removing service-settings from the DEBT ledger is required, not optional: the gate fails with "declares typecheck but still has a DEBT entry — it graduated; delete its entry". After deletion:

  • check:type-check-coverage: 63/77 → 64/77 packages type-checked; DEBT 14 → 13 entries; frozen raw errors 455 → 442. Self-test passes (23 semantic + 24 observation + 15 re-measure + 12 built-closure + 9 auto-lowering cases).
  • check:type-check-debt (--re-measure, against a fully built workspace): OK — 35 ledger entries re-measured, 1975 raw tsc errors total, none above its recorded number. Nothing raised.
  • check:query-options-erasure: holds — 67 unswept non-test sites in 17 files, none new; baseline key set verified against 6ff179d.
  • check:engine-double-contract: OK — 166 pinned, 133 DEBT, 2 exempt. No engine fake touched.
  • ESLint: clean over the changed surface (and green in CI).
  • vitest run in the package: 401 passed / 401, 19 files.

One pre-existing finding the re-measure surfaced (not caused by this PR, not fixed here)

The --re-measure pass reports 272 raw errors of surplus across 9 ledger entries sitting below their recorded ceiling — headroom in which regressions can land silently (#6376). None of them is service-settings, and nothing in this diff moved any of them:

metadata 92→89 · service-automation 5→3 · service-storage 52→51 (DEBT); plugin-approvals 547→348 · plugin-auth 131→108 · mcp 63→53 · lint 42→19 · plugin-security 21→11 · http-conformance 4→3 (TEST_DEBT).

pnpm check:type-check-debt --lower would close all nine in one write, but that rewrites nine ledger entries across eight unrelated packages on a one-package card — exactly the ballooning this card's scope forbids. Reported rather than done; it belongs to #6376.

#5536 ride-along: did not fire

The conditional trigger was packages/services/service-settings/src/settings-service.ts:1200. Clearing all 14 errors required no edit to settings-service.ts — the file is read-only in this work (its line 1809 is cited as evidence for the handler contract, nothing more). No single-point fix of storage hasAny; no promotion of #5536.

Not done, deliberately

…rrors the unexecuted gate hid (#7925)

`turbo run typecheck --filter=@objectstack/service-settings` exited 0 while
never running a typecheck task: the package declared only `build` and `test`,
and turbo silently no-ops a package missing the script. Adding it (mirroring
`service-messaging`) makes the task real — 7 tasks where there were 6 — and
surfaced 14 pre-existing errors in five test files.

All 14 are stale tests, not source defects; no non-test source file changed.
The manifest action-handler tests called handlers with a partial input while
`SettingsActionHandler` and every service call site pass
`{ namespace, actionId, values, payload?, ctx }`; the audit-sink test returned
`Array.push`'s number from a `void` callback; the translation-coverage guard
narrowed the manifests barrel through a hand-rolled type that had drifted from
the spec's `SettingsManifest`.

Nothing was silenced: no `any` added, no `@ts-expect-error`, tsconfig `include`
unchanged. The five `as any` casts already hiding the same drift in
`ai.manifest.test.ts` are removed rather than copied, and the
`aiTestEmbedderActionHandler` import the compiler flagged as unused is now
exercised — the `test_embedder` action button had no coverage.

`service-settings` graduates out of the `check:type-check-coverage` DEBT
ledger: 63/77 -> 64/77 packages type-checked, 455 -> 442 frozen raw errors.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ALEcHx72USrxEFv8Z2fKt1
@vercel

vercel Bot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 12, 2026 8:57am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

No hand-written docs reference the 0 changed package(s). ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

service-settings has no typecheck script — turbo silently no-ops it, and ~5 test files carry pre-existing type errors behind the unwired gate

1 participant