fix(core): a type that taught the wrong invariant — staleness signal column narrowed to legacy ids (#3159)

**Type-only. No runtime behaviour changes**, and I would rather say that
than let a green suite imply otherwise.

## The type described a guard that no longer exists

`TaskAgeStalenessSignal.column` was typed `"in-progress" | "in-review"`
and filled through a cast carrying this justification:

```ts
// The guard above proves `column` is one of these two legacy ids ... (#1403)
const activeColumn = task.column as "in-progress" | "in-review";
```

True when written. The guard now reads:

```ts
const wipColumn    = context.lifecycle?.wip    ?? "in-progress";
const reviewColumn = context.lifecycle?.review ?? "in-review";
if (task.column !== wipColumn && task.column !== reviewColumn) return undefined;
```

So on a renamed board it proves the column is `building` or `checking` —
and the cast asserted the **opposite** of what the guard established.
The runtime was always fine; the real id passed straight through.

## The damage is in what the type taught

A consumer writing `signal.column === "building"` got a **compile
error** saying the comparison was impossible. The type actively
instructed callers that `=== "in-progress"` is exhaustive — the exact
guard shape this program spends its time removing.

This is the second instance of the shape today. The first was
`dashboard/src/server.ts`:

```ts
moveTask(taskId: string, column: "todo", options?: …): Promise<unknown>;
```

which made the type system **reject** a resolved target (#3158). Neither
was a constraint anyone chose — both were inferred from a single legacy
call site and then hardened into an assertion about live data.

**A type narrowed to legacy ids is a lint against fixing the code**, and
it is invisible to every gate this program has: the census counts
comparisons, the move-target ratchet counts arguments, and neither looks
at type positions.

## The test is a characterization, and says so

No runtime test can differentiate a type-level fix — **`tsc` is what
differentiates it**. The added case pins a value that was already
correct, so a future narrowing has something to break against besides a
compile error nobody sees until they hit it. I have labelled it in the
file rather than presenting it as a regression test.

## Verification

| | result |
|---|---|
| `tsc` — core, engine, dashboard (app + src) | **0 errors** each |
| `task-age-staleness` | **17 passed** |
| all three staleness suites | **28 passed** |
| census `--strict` | exit 0 |

All three consumers of `.column` only display or compare it
(`taskAgeStalenessCopy.ts`, `TaskDetailModal`, a `TaskCard` memo
comparison), so nothing downstream narrows on the widened type.

## Scope note

I scanned for this class and the raw pattern is noisy — 192 candidates,
almost all object-literal **values** (`status: "archived"`), agent
roles, and unrelated `type: "done"` stream events. This one and
`server.ts` are the two I could confirm as genuine type-position
narrowings on a *task column*. I have not filed an issue for the class
because I cannot yet separate it from the noise reliably; if a cheap
discriminator turns up, it is worth a ratchet like the move-target one.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-31 07:06:48 -07:00
committed by GitHub
parent 44a67df65d
commit b9b7d14804
2 changed files with 56 additions and 5 deletions

View File

@@ -206,4 +206,33 @@ describe("age staleness resolves its lanes by ROLE", () => {
// With a 1ms wip warning and a ~999h review warning, only the wip threshold can produce critical.
expect(wipSignal?.level).toBe("critical");
});
/*
FNXC:WorkflowResolvedColumns 2026-07-31-23:59:
CHARACTERIZATION, and it is honest about being one: this pins the value the signal already emitted.
`TaskAgeStalenessSignal.column` was typed `"in-progress" | "in-review"` and filled through a cast
whose comment said "the guard above proves `column` is one of these two legacy ids". That stopped
being true when the guard was converted to compare against the RESOLVED wip/review lanes — from then
on the cast asserted the opposite of what the guard established, while the runtime happily passed the
renamed id through.
So widening the type changes NO behaviour, and no runtime test can differentiate it; the thing that
differentiates is `tsc`. Under the old narrow type a consumer writing `signal.column === "building"`
got a compile error telling them the comparison was impossible — the type actively taught the wrong
invariant. This case exists so the value is at least pinned, and so a future narrowing has something
to break against besides a type error nobody sees until they hit it.
The `as never` casts on the fixtures below are the same shape and stay: `ColumnId` is a union with a
`string & {}` member, and these are deliberately ids no board declares.
*/
it("reports the RENAMED lane id it matched, not a legacy one", () => {
const signal = getTaskAgeStalenessSignal(
{ ...baseTask, column: "building" as never, columnMovedAt: STALE_MOVED_AT },
{ now: NOW, lifecycle: RENAMED },
);
expect(signal).toBeDefined();
expect(signal?.column).toBe("building");
});
});

View File

@@ -9,7 +9,21 @@ export interface TaskAgeStalenessSignal {
ageMs: number;
warningThresholdMs: number;
criticalThresholdMs: number;
column: "in-progress" | "in-review";
/*
FNXC:WorkflowResolvedColumns 2026-07-31-23:59:
WIDENED from `"in-progress" | "in-review"`, because the guard that fills it was converted and this
type was left describing the old one.
`getTaskAgeStalenessSignal` resolves the lanes it accepts (`context.lifecycle?.wip ?? …`), so on a
renamed board this field legitimately holds `building` or `checking`. The narrow type therefore
asserted something FALSE about live data, and forced a cast to keep saying it — see the note at the
assignment.
A type narrowed to legacy ids is not a harmless leftover: it tells every consumer that a
`=== "in-progress"` comparison is exhaustive, which is exactly the guard this program spends its
time removing. All three consumers today only display or compare the value, so widening is safe.
*/
column: string;
paused: boolean;
}
@@ -63,10 +77,18 @@ export function getTaskAgeStalenessSignal(
if (task.column !== wipColumn && task.column !== reviewColumn) {
return undefined;
}
// The guard above proves `column` is one of these two legacy ids; the
// `ColumnId` union's `string & {}` member can't be excluded by literal `!==`
// narrowing, so the cast is provably safe here (#1403).
const activeColumn = task.column as "in-progress" | "in-review";
/*
FNXC:WorkflowResolvedColumns 2026-07-31-23:59:
THE CAST IS GONE, and the comment it carried had become false.
It read: "the guard above proves `column` is one of these two legacy ids" (#1403). That was true
when the guard compared against the literals. The guard now compares against `wipColumn` /
`reviewColumn`, which are RESOLVED — so on a renamed board it proves the column is `building` or
`checking`, and the cast was asserting the opposite of what the guard established.
Nothing needs casting now that the field's type matches what the guard admits.
*/
const activeColumn = task.column;
if (task.mergeDetails?.mergeConfirmed === true) {
return undefined;
}