`batch-cli-plugins` — the u7 worker's mega-batch: `packages/cli` + `plugins` + anything left. ## The batch is 7 guards, and 3 of them are not guards at all The census's per-file list gives this batch seven sites. Reading them, **three are a foreign vocabulary the census matches on the string alone**: | file | site | verdict | |---|---|---| | `plugins/fusion-plugin-reports/store/report-store.ts` | `next === "archived"` ×2 | **not a column** — `next` is a `ReportStatus` | | `plugins/fusion-plugin-reports/store/report-types.ts` | `to === "failed" \|\| to === "archived"` | **not a column** — same enum, its own terminal states | The reports plugin has its own status lineage (`draft → generating → review_* → approved → published`, plus `failed`/`archived`) that shares two spellings with the lifecycle vocabulary. A report is not on a board and has no workflow, so resolving an IR there would answer a question nobody asked. All three are marked `DELIBERATE-LITERAL` with the reason at the site. **This cuts the other way from #2763.** That PR establishes the census total as a *floor* (25 membership predicates it structurally cannot see). This is the opposite error in the same number: a foreign enum inflating it. The total is neither a ceiling nor a floor — it is an estimate with error in both directions, and the per-file list is worth reading before trusting a file's count. ## Converted (census before → after, per file) | file | before | after | |---|---|---| | `packages/cli/src/commands/pr.ts` | 1 | **0** | | `plugins/…/even-realities-glasses/notifications/diff.ts` | 1 | **0** | | `plugins/…/reports/store/report-store.ts` | 2 | **0** (deliberate) | | `plugins/…/reports/store/report-types.ts` | 1 | **0** (deliberate) | ### `fn pr create` refused every card on a renamed board The live defect in this batch. The gate was `task.column !== "in-review"`, and its error told the operator to move the task to a column their board does not have: ``` Error: Task must be in 'in-review' column to create a PR (current: signoff) ``` There is no way to satisfy that short of renaming the workflow back. Now resolved through core's `resolveReviewColumns`, and the message names the lanes that actually exist. **The SET, not `lifecycle.review`.** A board may declare more than one review lane, and a card parked in a `humanReview`-only lane is still a card you can open a PR from. A single-id answer keeps refusing those — the same narrowing #2728's review caught in the CLI retry gate, which is why the test pins both lanes. ## Skipped, with the reason **`plugins/fusion-plugin-even-cards` (2 guards) — blocked on packaging, not on analysis.** The defect is real: `boardToDeck` filters with `column !== "archived" && column !== "done"`, so on a renamed board every finished card stays in the deck, fills `maxCards`, and pushes the active cards off the display. The wearer sees a board that never finishes anything. I implemented the fix and **reverted it**: this plugin is not in `pnpm-workspace.yaml` and depends only on `@fusion/plugin-sdk` — it has no `@fusion/core` dependency, so the route cannot reach `resolveTaskLifecycleColumns`. Adding one is a packaging change, which this program's rules put out of scope. Shipping only the injected parameter without a caller was the alternative, and that is precisely the decorative conversion #2759 documents: the census would drop by 2 and the deck would keep the bug. Flagged for whoever owns the plugin's dependency surface. The glasses plugin next door *does* depend on `@fusion/core`, so this is a one-plugin problem, not a plugin-wide one. ## Honest note on the glasses conversion `diff.ts`'s completion branch is **currently unreachable** — the only production caller (`notifier.ts`) passes `alsoNotifyOnDone: false`. So that conversion changes nothing at runtime today. It is converted rather than marked deliberate because the literal is not deliberate: it is wrong, and would ship the bug the day someone turns the flag on. Stated here rather than left for a reviewer to discover. ## Verification - new CLI suite **4 passed**; `pr-command` + `pr-automerge-cleanup` + `bin-pr-router` **35 passed** - glasses plugin **181 passed (19 files)** · reports plugin **110 passed (23 files)** - `pnpm test:gate` — **158 / 10 / 487 / 71** · `pnpm lint` clean · `--strict` exits 0 **Revert proof, measured.** Restoring `if (task.column !== "in-review")` fails 3 of the 4 new cases (`process.exit:1` on both renamed lanes, and the refusal message reverts to naming `in-review`). The unresolvable-workflow case keeps passing — it is the legacy path — so the negative cases alone do not pin the fix and all four are required. ## Handoff to `batch-engine` `packages/engine/src/project-engine.ts` **5 → 0** is finished, green, and pushed as `handoff/project-engine-lanes-for-batch-engine` (`34dbb35209`) for the capacity worker to cherry-pick — it is engine-owned, not mine to land. It fixes two live defects: a card that **had merged** reported as a failed merge to `fn task merge` and the dashboard button (`merged: finalTask?.column === "done"`), and the three post-finalize `column === "done" && mergeConfirmed` fast-path checks, which on a renamed board sent an already-landed card down the bounce path — re-queued, retry-counted, and in the capped branch parked `failed` with its merge sitting on main. Plus `hasAutoHealableVerificationBufferFailure`, which returned false for every card on a renamed board, so a buffer-overflow verification failure was never auto-healed. 8 new tests, revert-proven (restoring the literal fails 4 of 8), gate green. --- ## Completion pass (u7) — the batch is now closed Two workers converged on this branch. I rebased onto the first-landed commit rather than force-pushing over it, took its wording wherever the conclusion was identical, and added what was missing. ### What this pass added 1. **`even-cards` (2 sites)** — the only in-scope file the first pass left open. Marked DELIBERATE-LITERAL: the package depends on `@fusion/plugin-sdk` only, and the SDK does not re-export the lifecycle role helpers, so there is no IR, no store, and no trait flags to resolve *from*. Fixing it properly means the SDK exposing role flags on the task shape it hands plugins — a structural change, out of scope, and recorded at the site as the correct home. Live consequence is cosmetic: a finished card on a renamed board shows as active in the glasses deck. 2. **A red test in the `fn pr create` conversion.** The incoming version rendered `Task must be in 'in-review' to create a PR`, dropping the word `column`. `task.test.ts:3422` pins `must be in 'in-review' column`, so that hunk failed `runTaskPrCreate > exits with error when task not in in-review column`. Restoring the word makes the single-lane message **byte-identical** to the pre-conversion one, which is what a vocabulary conversion should be — the guard's own test now passes unmodified. Marked at the site so it is not "simplified" back. 3. **Duplicate imports** — the two independent conversions each added `resolveWorkflowIrForTask`/`resolveReviewColumns`, which does not compile. Deduped in its own commit. ### Census Measured with `--json` on `origin/main` and on this branch. | file | before | after | action | |---|---|---|---| | `packages/cli/src/commands/pr.ts` | 1 | 0 | converted | | `plugins/fusion-plugin-reports/src/store/report-types.ts` | 1 | 0 | marked | | `plugins/fusion-plugin-reports/src/store/report-store.ts` | 2 | 0 | marked | | `plugins/fusion-plugin-even-cards/src/cards/board-cards.ts` | 2 | 0 | marked | | `plugins/fusion-plugin-even-realities-glasses/.../diff.ts` | 1 | 0 | marked | Backlog **415 → 408** (−7, exactly the in-scope count). Deliberate **40 → 46** (+6 marked); 6 + 1 converted = 7. `--strict` exits 0. **Nothing remains in `cli` + `plugins` + everything-else — there is no follow-up batch behind this one.** ### One note on the `even-realities-glasses` site Worth recording beyond "cannot resolve": its only production caller (`notifier.ts:80`) passes `alsoNotifyOnDone: false`, so that arm is **unreachable today**. Converting it could not have changed observed behaviour either way. ### Verification (measured, on the merged branch) - `pnpm --filter @runfusion/fusion exec tsc --noEmit` → exit 0 - `pnpm lint` → 0 errors - CLI `task.test.ts` → 144 passed, including the `runTaskPrCreate` guard test - `@fusion-plugin-examples/reports` → 110 passed; `even-realities-glasses` → 181 passed **Pre-existing failures, not from this change:** the 5 `runTaskImportFromGitHub` / `runTaskImportGitHubInteractive` tests fail identically on `origin/main` — verified by stashing this diff and re-running (5 failed / 144 passed both ways). --- ## Census audit (unowned follow-on) After closing the batch scope I audited whether the **392** column-backlog number is inflated by foreign vocabularies — the class this batch found in the reports plugin, where `"archived"` is a `ReportStatus` rather than a board lane. If that class were widespread, every remaining batch would be chasing sites that must not be converted. **It is not. The number is real.** A receiver-level pass over all 392 column-category sites found exactly **3** false positives, all in `plugins/fusion-plugin-reports` (`next`, a `ReportStatus`), all now marked in this PR. What was checked and cleared: - **Property-reached foreign enums** (`step.status`, `feature.status`, `mission.status`) — already correctly bucketed into the separate `status` category (185), not the column backlog. Verified against `merge-queue-ops.ts`: 11 lifecycle-spelled literals in the file, census counts **1**, and that 1 is the genuine `.column` guard. - **Bare step-status variables** (`status`, `currentStatus`, `liveStatus` compared to `"done"`/`"skipped"`) — likewise excluded. - **Every other receiver in the backlog** — `to`, `from`, `column`, `fromColumn`, `toColumn`, `latestColumn`, `state`, `preArchiveColumn`. All resolve to genuine task columns. `executor.ts`'s 15 sites were spot-checked line by line: all 15 are real. The gap the classifier genuinely cannot close is a foreign enum held in a **bare variable** — the receiver name carries no type information, so `next === "archived"` is indistinguishable from a lifecycle guard by AST alone. That is why the reports sites need a marker rather than a classifier fix, and it is now documented in `lifecycle-column-census-ast.mjs`'s header alongside the measured scope, so the remaining batches do not re-run this hunt. Census tests: **43 passed**. The change is comment-only. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
504 lines
25 KiB
JavaScript
504 lines
25 KiB
JavaScript
/*
|
|
FNXC:WorkflowLifecycleColumns 2026-07-30-22:20 (Phase C convergence — AST classifier):
|
|
|
|
WHY AN AST AND NOT A REGEX. Three people measured the remaining lifecycle-column work with three
|
|
greps and got three answers (6, 8, 12 role-bucket sites). A regex cannot tell a lifecycle-column
|
|
comparison from an agent role, a session purpose, a surface name, a step status, or a comment — so
|
|
no grep-derived number is authoritative, however careful the pattern. This module parses instead.
|
|
|
|
WHAT THE PARSER BUYS, concretely, over the text census next to it:
|
|
- comments are not tokens, so prose about an old guard cannot be counted (the text version needed
|
|
a comment stripper, and a bug in that stripper let ONE marker launder FOUR live guards);
|
|
- the receiver is a real expression, so `t.column`, `live?.column`, `String(task.status)` and
|
|
`tasks[i].column` all resolve without a hand-tuned pattern per shape;
|
|
- sibling comparisons are found by walking the ENCLOSING expression rather than a line window, so
|
|
a multi-line `||` chain is one unit and an unrelated line four rows away is not.
|
|
|
|
WHAT IT STILL CANNOT DO, stated plainly rather than implied: without a full type-checker program it
|
|
cannot prove a receiver is column-typed. So classification remains evidence-based — the receiver's
|
|
name plus the vocabulary its siblings use — and the three non-column classes are reported
|
|
SEPARATELY rather than netted, so a wrong classification is visible instead of silently changing
|
|
the bar. Two independent implementations agreeing on 12 role sites is the strongest evidence
|
|
available; one number from one grep is the weakest.
|
|
|
|
CLASSES (only the first is backlog):
|
|
column — a lifecycle-column guard.
|
|
role — AgentRole / session purpose / surface. Converting one is a real bug: the planner
|
|
LANE is named `triage` and keeps that name; U11 removed the COLUMN.
|
|
status — StepStatus / mission / goal / feature status. `done`, `in-progress` and `archived`
|
|
collide with column ids; `pending` and `skipped` never do.
|
|
deliberate — reviewed literal carrying a DELIBERATE-LITERAL marker in its leading comments.
|
|
|
|
FOREIGN VOCABULARIES (audited 2026-07-30, batch-cli-plugins).
|
|
|
|
The `status` category above catches a foreign enum reached through a PROPERTY (`step.status`,
|
|
`feature.status`). It cannot catch one held in a BARE VARIABLE — `if (next === "archived")`, where
|
|
`next` is a ReportStatus — because the receiver name carries no type information. Those land in the
|
|
`column` backlog and read as unconverted lifecycle guards.
|
|
|
|
Do NOT convert them: resolving a report's status against a task workflow is a category error. Mark
|
|
the site DELIBERATE-LITERAL with the owning vocabulary named, as the reports plugin now does.
|
|
|
|
Measured scope, so nobody re-hunts this: a full receiver-level audit of all 392 column-category
|
|
sites found exactly 3 such false positives, all in `plugins/fusion-plugin-reports` (`next`, a
|
|
ReportStatus), and all now marked. Every other receiver sampled — `to`, `from`, `column`,
|
|
`fromColumn`, `toColumn`, `latestColumn`, `state`, `preArchiveColumn` — resolved to a genuine task
|
|
column. Bare `status`/`currentStatus`/`liveStatus` step comparisons are correctly excluded already
|
|
(`merge-queue-ops.ts` counts 1 of its 11 literals, and that 1 is the real `.column` guard).
|
|
|
|
The backlog number is therefore real. Treat a surprising count as work, not as noise.
|
|
*/
|
|
|
|
import { createRequire } from "node:module";
|
|
import { readFileSync } from "node:fs";
|
|
|
|
const require = createRequire(import.meta.url);
|
|
const ts = require("typescript");
|
|
|
|
/** The legacy lifecycle column vocabulary — the ids that shipped as the builtin board. */
|
|
export const LEGACY_COLUMN_IDS = ["triage", "todo", "in-progress", "in-review", "done", "archived"];
|
|
|
|
/** Receiver names that denote an agent role / lane rather than a task column. */
|
|
export const ROLE_RECEIVER_TOKENS = [
|
|
"role", "agentType", "agent", "lane", "capability", "sessionPurpose", "surface", "purpose", "agentRole",
|
|
/*
|
|
FNXC:LifecycleColumnCensus 2026-07-30-22:00 (fleet phase — the work order was sending workers at
|
|
non-columns):
|
|
EVENT/RESULT DISCRIMINATORS, not columns. Found while claiming TaskDetailModal.tsx, whose census
|
|
entry included `session.agentState === "done"` — an agent state, not a lane. Auditing every
|
|
receiver the classifier currently counts surfaced four more of the same shape:
|
|
|
|
event.type === "done" register-chat-routes.ts — an SSE event type
|
|
mode === "done" useTaskDiffStats.ts — a cache-key mode
|
|
evidence.kind === "done" async-mission-store.ts — an evidence kind
|
|
event.kind === "done" telemetry-hub.ts — a telemetry event kind
|
|
|
|
Each shares a WORD with a column id and nothing else. Converting one asks the trait registry
|
|
what lane an SSE event is in, which has no answer — the same failure class as converting
|
|
`role === "triage"`, which this list already exists to prevent.
|
|
|
|
`state` is deliberately NOT added: `state === "archived"` in audit-ops/comments-ops is a task's
|
|
column reaching those functions under a shorter name, so it is a genuine guard. Checked rather
|
|
than assumed, because excluding a real one silently lowers the bar.
|
|
*/
|
|
"type", "mode", "kind", "phase", "agentState",
|
|
/*
|
|
FNXC:LifecycleColumnCensus 2026-07-29-20:50 (restores the pinned baseline):
|
|
`outcome` names a RESULT enum, not a column. The one live instance is
|
|
`deterministicReconcile.outcome === "archived"` — the verdict of a duplicate reconciliation, which
|
|
happens to share a word with a column id.
|
|
|
|
This is not a preference: the shipped classifier counted it, the pinned baseline did not, and that
|
|
single site is the entire 22-vs-23 gap that has kept `--strict` RED on main since #2633 merged.
|
|
So the baseline was recorded by a classifier that excluded it, and the exclusion was lost before
|
|
the code shipped. Restoring it makes the instrument agree with its own pin rather than raising the
|
|
pin to match a miscount — which would have quietly conceded a guard that does not exist.
|
|
*/
|
|
"outcome",
|
|
];
|
|
|
|
/*
|
|
Values that belong to ONE vocabulary only, and therefore identify which vocabulary an expression is
|
|
matching regardless of what its variable is called. `AgentRole` is `triage | executor | reviewer |
|
|
merger` and `StepStatus` is `pending | in-progress | done | skipped`; the members below are never
|
|
column ids. This is the signal that caught `sessionPurpose` and `surface`, which a name list missed.
|
|
*/
|
|
const ROLE_ONLY_VALUES = new Set(["executor", "reviewer", "merger"]);
|
|
const STATUS_ONLY_VALUES = new Set([
|
|
"pending", "skipped",
|
|
/*
|
|
FNXC:LifecycleColumnCensus 2026-07-29-21:40 (widen the sibling vocabulary, not the name list):
|
|
Members of state/phase/result enums that are NEVER column ids. Each earns its place by a measured
|
|
site whose siblings prove the vocabulary:
|
|
|
|
stepState { active, done } DashboardLoader.tsx:123
|
|
agentState { busy, ready, starting, done } TaskDetailModal.tsx:339
|
|
phase { confirm, pushing, done } dashboard-tui/app.tsx:3139
|
|
kind { exhausted, existing, invalid-deleted, missing, async-mission-store.ts:1175
|
|
nonterminal, stopped, done }
|
|
|
|
Deliberately extending the VALUE vocabulary rather than the receiver-name list, because names are
|
|
unreliable here and provably so: `state` looked like the same class but holds
|
|
`await getLiveTaskColumn(...)` — a real column, correctly counted. A name rule would have deleted
|
|
that guard from the backlog. The sibling signal is the mechanism that already caught
|
|
`sessionPurpose` and `surface`.
|
|
*/
|
|
"active", "busy", "ready", "starting", "confirm", "pushing",
|
|
"exhausted", "existing", "invalid-deleted", "missing", "nonterminal", "stopped",
|
|
]);
|
|
|
|
export const DELIBERATE_MARKER = "DELIBERATE-LITERAL";
|
|
|
|
const COMPARISON_KINDS = new Set([
|
|
ts.SyntaxKind.EqualsEqualsEqualsToken,
|
|
ts.SyntaxKind.ExclamationEqualsEqualsToken,
|
|
ts.SyntaxKind.EqualsEqualsToken,
|
|
ts.SyntaxKind.ExclamationEqualsToken,
|
|
]);
|
|
|
|
/** The name a comparison is made against: the property, the identifier, or the callee's argument. */
|
|
function receiverNameOf(node) {
|
|
if (ts.isPropertyAccessExpression(node)) return node.name.getText();
|
|
if (ts.isElementAccessExpression(node)) return receiverNameOf(node.expression);
|
|
if (ts.isIdentifier(node)) return node.getText();
|
|
if (ts.isNonNullExpression(node) || ts.isParenthesizedExpression(node) || ts.isAsExpression(node)) {
|
|
return receiverNameOf(node.expression);
|
|
}
|
|
// `String(task.status)` / `normalize(col)` — the interesting name is the argument's.
|
|
if (ts.isCallExpression(node) && node.arguments.length === 1) return receiverNameOf(node.arguments[0]);
|
|
return "";
|
|
}
|
|
|
|
/** The string literal side of a comparison, if exactly one side is one. */
|
|
function literalOf(binary) {
|
|
const left = binary.left;
|
|
const right = binary.right;
|
|
const leftIsLiteral = ts.isStringLiteralLike(left);
|
|
const rightIsLiteral = ts.isStringLiteralLike(right);
|
|
if (leftIsLiteral === rightIsLiteral) return undefined;
|
|
return leftIsLiteral
|
|
? { literal: left.text, receiver: right }
|
|
: { literal: right.text, receiver: left };
|
|
}
|
|
|
|
/**
|
|
* The outermost expression this comparison participates in, so a multi-line `||` chain is examined
|
|
* as ONE unit. A line window cannot express that: it both misses long chains and pulls in
|
|
* unrelated neighbours.
|
|
*/
|
|
function enclosingExpression(node) {
|
|
let current = node;
|
|
while (
|
|
current.parent
|
|
&& (ts.isBinaryExpression(current.parent)
|
|
|| ts.isParenthesizedExpression(current.parent)
|
|
|| ts.isPrefixUnaryExpression(current.parent)
|
|
|| ts.isConditionalExpression(current.parent))
|
|
) {
|
|
current = current.parent;
|
|
}
|
|
return current;
|
|
}
|
|
|
|
/** Every string literal compared against `receiverName` inside `scope`. */
|
|
function siblingLiteralsFor(scope, receiverName) {
|
|
const values = new Set();
|
|
const visit = (node) => {
|
|
if (ts.isBinaryExpression(node) && COMPARISON_KINDS.has(node.operatorToken.kind)) {
|
|
const parts = literalOf(node);
|
|
if (parts && receiverNameOf(parts.receiver) === receiverName) values.add(parts.literal);
|
|
}
|
|
ts.forEachChild(node, visit);
|
|
};
|
|
visit(scope);
|
|
return values;
|
|
}
|
|
|
|
/** True when a DELIBERATE-LITERAL marker appears in the comments attached above this node. */
|
|
function hasDeliberateMarker(sourceFile, node) {
|
|
const fullText = sourceFile.getFullText();
|
|
/*
|
|
Walk every ANCESTOR, not just the enclosing statement. The real markers in this codebase sit above
|
|
the enclosing FUNCTION (`legacyDependencySatisfied` in hold-release.ts is the case that caught
|
|
this) while the comparison is a return statement inside it — so a statement-only lookup found
|
|
nothing and silently reclassified three reviewed literals as backlog.
|
|
|
|
Ancestor scope is also the right SEMANTICS, and strictly tighter than the line window it replaces:
|
|
a marker excuses the construct it is attached to and everything inside it, and nothing else. The
|
|
window version excused whatever happened to be within twelve lines.
|
|
*/
|
|
let current = node;
|
|
while (current && !ts.isSourceFile(current)) {
|
|
const ranges = ts.getLeadingCommentRanges(fullText, current.getFullStart()) ?? [];
|
|
if (ranges.some((range) => fullText.slice(range.pos, range.end).includes(DELIBERATE_MARKER))) {
|
|
return true;
|
|
}
|
|
current = current.parent;
|
|
}
|
|
return false;
|
|
}
|
|
|
|
/*
|
|
FNXC:LifecycleColumnCensus 2026-07-29-19:20 (query-filter category):
|
|
|
|
A guard is not the only way a legacy column id decides behaviour. `listTasks({ column: "todo" })`
|
|
is a SOURCE QUERY: it selects the rows a sweep will consider, and on a board that renamed or merged
|
|
that column it returns nothing — so a sweep whose per-task predicate was correctly converted still
|
|
does nothing, and looks converted while being dead. `self-healing.ts:2849` names the pairing in
|
|
prose, and #2560 had to repair exactly that combination after a converted predicate was left with a
|
|
literal query. One measured consequence: `recoverStuckMergeDeadlocks` cannot see a renamed board at
|
|
all (proven on a live store: the renamed rows exist and none appear in its three-literal union).
|
|
|
|
The comparison walk cannot see these — a PropertyAssignment is not a BinaryExpression — so they
|
|
were invisible to the census and to its ratchet, meaning the class could grow silently.
|
|
|
|
COUNTED SEPARATELY, deliberately. `totals.column` and the per-column/per-file backlog are left
|
|
byte-identical, so the completion bar ("triage guards to 0") keeps its existing meaning and the
|
|
pinned baseline does not move. This adds a second, independently pinned number.
|
|
|
|
DEFINITIONS ARE NOT QUERIES. Workflow IR graph nodes carry `column:` to declare where a node lives
|
|
(`{ id: "review", kind: "...", column: "in-review" }`), which is the lineage DEFINING itself — the
|
|
builtin IR files hold ~32 of these. Converting one would be nonsense. They are told apart
|
|
structurally rather than by filename: a definition's object literal also carries `id:` or `kind:`,
|
|
a query's does not.
|
|
*/
|
|
function classifyColumnProperty(node) {
|
|
const object = node.parent;
|
|
if (!object || !ts.isObjectLiteralExpression(object)) return "query";
|
|
const hasDefinitionSibling = object.properties.some(
|
|
(property) =>
|
|
property !== node
|
|
&& ts.isPropertyAssignment(property)
|
|
&& ts.isIdentifier(property.name)
|
|
&& (property.name.text === "id" || property.name.text === "kind"),
|
|
);
|
|
return hasDefinitionSibling ? "definition" : "query";
|
|
}
|
|
|
|
/** True for a `column: "<legacy id>"` property assignment. */
|
|
function columnPropertyLiteral(node) {
|
|
if (!ts.isPropertyAssignment(node)) return undefined;
|
|
const name = ts.isIdentifier(node.name) || ts.isStringLiteral(node.name) ? node.name.text : undefined;
|
|
if (name !== "column") return undefined;
|
|
if (!ts.isStringLiteral(node.initializer)) return undefined;
|
|
return LEGACY_COLUMN_IDS.includes(node.initializer.text) ? node.initializer.text : undefined;
|
|
}
|
|
|
|
|
|
/*
|
|
FNXC:LifecycleColumnCensus 2026-08-01-01:10:
|
|
A LEGACY LITERAL IN A FALLBACK BRANCH IS NOT BACKLOG. The converted shape across the dashboard is
|
|
|
|
if (flags) return flags.hold === true || flags.countsTowardWip === true;
|
|
return column === "todo" || column === "in-progress"; // <- reachable only without traits
|
|
|
|
and that second literal is CORRECT: it answers for callers that have no resolved column metadata, which is
|
|
the distinction `resolveLifecycleColumns` returns `undefined`-for-the-whole-struct to preserve. Counting it
|
|
as an unconverted guard tells a batch worker to convert code that is already converted — and "convert it"
|
|
there means deleting the only answer available when traits are absent.
|
|
|
|
MEASURED, which is why this is worth a class rather than a preference: a proximity scan for
|
|
"legacy literal near a role-resolved call" over the dashboard returned 19 hits and ZERO defects, every one
|
|
this shape. The same scan over the engine found two real defects (#2670, #2672) — and in BOTH the literal was
|
|
in a separate statement beside resolved data, not in a fallback branch. That is the whole difference, and it
|
|
is structural, so the parser can see it.
|
|
|
|
Reported as its own line rather than removed from the total: a fallback literal is still a literal, and the
|
|
day the trait path is unconditional it should be deleted. This distinguishes "not yet converted" from
|
|
"converted, with a documented degradation".
|
|
*/
|
|
const TRAIT_TEST_HINTS = [
|
|
"flags", "Flags", "columnFlags", "lifecycle", "roles", "trait", "resolveColumnFlags", "columnHasFlag",
|
|
"resolveLifecycleColumns", "intake", "hold", "countsTowardWip", "mergeOrchestration", "mergeBlocker",
|
|
];
|
|
|
|
/*
|
|
FNXC:LifecycleColumnCensus 2026-07-30-10:40 (PR #2677 review — coderabbit):
|
|
HINTS MUST NOT MATCH INSIDE A LONGER IDENTIFIER. The leading class was `[.?\w]`, so the `hold`
|
|
hint matched `threshold`, `staleThreshold`, `household`, `stronghold`, `withhold` — and `flags`
|
|
matched `myflags`. Any branch testing an unrelated `threshold` was then read as testing resolved
|
|
trait data, which marks a live legacy guard as an already-converted fallback.
|
|
|
|
The `\w` was also REDUNDANT, which is why removing it costs nothing: the third alternative
|
|
`\bhold\b` already matches `flags.hold` and `flags?.hold`, because `.` is a non-word character
|
|
and so supplies the word boundary itself. The `\w` alternative added only the false positives.
|
|
*/
|
|
/** True when `text` reads as a test for resolved trait data rather than for a column name. */
|
|
function testsTraitData(text) {
|
|
return TRAIT_TEST_HINTS.some((hint) => new RegExp(`[.?]${hint}\\b|\\b${hint}\\s*[?.]|\\b${hint}\\b`).test(text))
|
|
&& !new RegExp(`(===|!==)\\s*["'](${LEGACY_COLUMN_IDS.join("|")})["']`).test(text);
|
|
}
|
|
|
|
/*
|
|
FNXC:LifecycleColumnCensus 2026-07-30-10:05 (PR #2677 review — greptile):
|
|
A RETURN TOKEN IS NOT TERMINATION. The early-return detector used to ask whether the `then`
|
|
branch CONTAINED the word `return` anywhere. A branch that only returns conditionally —
|
|
`if (flags) { if (x) return a; }` — satisfies that while still falling through, so the
|
|
literal after it is REACHABLE with traits present and is a live guard.
|
|
|
|
The misclassification runs in the dangerous direction: it removes a real guard from the
|
|
backlog the census is trusted to report, and it does so silently. This asks whether the
|
|
branch DEFINITELY terminates instead, which is a property of structure rather than of the
|
|
presence of a token.
|
|
*/
|
|
function alwaysTerminates(stmt) {
|
|
if (!stmt) return false;
|
|
if (ts.isReturnStatement(stmt) || ts.isThrowStatement(stmt)) return true;
|
|
if (ts.isBlock(stmt)) {
|
|
const statements = stmt.statements ?? [];
|
|
return statements.length > 0 && alwaysTerminates(statements[statements.length - 1]);
|
|
}
|
|
/* Only terminates when BOTH arms do — a missing else is exactly the fall-through case. */
|
|
if (ts.isIfStatement(stmt)) {
|
|
return alwaysTerminates(stmt.thenStatement) && alwaysTerminates(stmt.elseStatement);
|
|
}
|
|
return false;
|
|
}
|
|
|
|
/**
|
|
* True when this comparison sits in the FALLBACK branch of a conditional whose test reads resolved
|
|
* trait data — i.e. it is the documented answer for callers without traits, not an unconverted guard.
|
|
*/
|
|
function isTraitFallback(node, sourceFile) {
|
|
let current = node;
|
|
while (current.parent && !ts.isSourceFile(current.parent)) {
|
|
const parent = current.parent;
|
|
if (ts.isConditionalExpression(parent) && parent.whenFalse === current) {
|
|
if (testsTraitData(parent.condition.getText(sourceFile))) return true;
|
|
}
|
|
if (ts.isIfStatement(parent)) {
|
|
/*
|
|
Two shapes count: the explicit `else`, and the EARLY-RETURN form — `if (flags) return ...;` followed by
|
|
the literal as the next statement, which is how most of these are actually written. The early-return
|
|
case is detected by looking at the preceding sibling statement rather than at an else branch.
|
|
*/
|
|
if (parent.elseStatement === current && testsTraitData(parent.expression.getText(sourceFile))) return true;
|
|
}
|
|
if (ts.isBlock(parent) || ts.isSourceFile(parent)) {
|
|
const statements = parent.statements ?? [];
|
|
const index = statements.indexOf(current);
|
|
for (let i = index - 1; i >= 0 && i >= index - 2; i -= 1) {
|
|
const prior = statements[i];
|
|
if (ts.isIfStatement(prior)
|
|
&& prior.elseStatement === undefined
|
|
&& testsTraitData(prior.expression.getText(sourceFile))
|
|
&& alwaysTerminates(prior.thenStatement)) {
|
|
return true;
|
|
}
|
|
}
|
|
}
|
|
current = parent;
|
|
}
|
|
return false;
|
|
}
|
|
|
|
/** Parse one file and classify every comparison against a legacy column id. */
|
|
export function findComparisons(filePath, source) {
|
|
const sourceFile = ts.createSourceFile(filePath, source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX);
|
|
const findings = [];
|
|
|
|
const visit = (node) => {
|
|
if (ts.isBinaryExpression(node) && COMPARISON_KINDS.has(node.operatorToken.kind)) {
|
|
const parts = literalOf(node);
|
|
if (parts && LEGACY_COLUMN_IDS.includes(parts.literal)) {
|
|
const receiver = receiverNameOf(parts.receiver);
|
|
const siblings = siblingLiteralsFor(enclosingExpression(node), receiver);
|
|
const isRole = ROLE_RECEIVER_TOKENS.includes(receiver)
|
|
|| [...siblings].some((value) => ROLE_ONLY_VALUES.has(value));
|
|
const isStatus = /status/i.test(receiver)
|
|
|| [...siblings].some((value) => STATUS_ONLY_VALUES.has(value));
|
|
const deliberate = hasDeliberateMarker(sourceFile, node);
|
|
findings.push({
|
|
file: filePath,
|
|
line: sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)).line + 1,
|
|
columnId: parts.literal,
|
|
receiver,
|
|
kind: deliberate ? "deliberate" : isRole ? "role" : isStatus ? "status" : "column",
|
|
/*
|
|
Advisory only — it never changes `kind`, so a wrong hint cannot move the bar. It exists so a batch
|
|
worker can tell an unconverted guard from an already-converted site's documented fallback.
|
|
*/
|
|
traitFallback: isTraitFallback(node, sourceFile),
|
|
});
|
|
}
|
|
}
|
|
const columnProperty = columnPropertyLiteral(node);
|
|
if (columnProperty) {
|
|
findings.push({
|
|
file: filePath,
|
|
line: sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)).line + 1,
|
|
columnId: columnProperty,
|
|
receiver: "column",
|
|
kind: hasDeliberateMarker(sourceFile, node) ? "deliberate" : classifyColumnProperty(node),
|
|
});
|
|
}
|
|
ts.forEachChild(node, visit);
|
|
};
|
|
visit(sourceFile);
|
|
|
|
return findings;
|
|
}
|
|
|
|
/** Aggregate findings into the four headline counts plus per-file and per-column breakdowns. */
|
|
export function summarize(findings) {
|
|
/*
|
|
FNXC:LifecycleColumnCensus 2026-08-01-01:55:
|
|
Reported ALONGSIDE `totals`, not inside it. `totals` is the four-class contract other suites deep-equal, so
|
|
adding a key there breaks assertions that are correctly strict about the shape — my first attempt did
|
|
exactly that and failed two existing cases. An advisory number does not belong in the structure that
|
|
defines the bar.
|
|
*/
|
|
let traitFallbackCount = 0;
|
|
const totals = { column: 0, role: 0, status: 0, deliberate: 0 };
|
|
const byColumnId = {};
|
|
const byFile = new Map();
|
|
|
|
/*
|
|
Kept OUT of `totals` on purpose. `totals` is a published shape: the baseline file, the reporter,
|
|
and other workers' in-flight PRs all read it, and the completion bar is defined against
|
|
`totals.column`. Growing that object would move a number people are mid-way through driving to
|
|
zero. The property-assignment counts are a second, independent instrument and live beside it.
|
|
*/
|
|
/*
|
|
FNXC:WorkflowLifecycleColumns 2026-07-30-09:00 (PR #2661 review — greptile P1):
|
|
DELIBERATE counts are tracked PER FILE, not only as a repo total. A total lets an addition in one
|
|
marked construct be offset by a removal in another and stay flat, and because deliberate findings
|
|
are excluded from `byFile`, the newly exempt guard is invisible there too — so the gate passes with
|
|
a new lifecycle-column guard. Per-file is the same shape `byFile` already uses for columns, and it
|
|
makes offsetting edits visible because they land in different files.
|
|
*/
|
|
const deliberateByFile = new Map();
|
|
|
|
const properties = { query: 0, definition: 0 };
|
|
const queryByFile = new Map();
|
|
const queryByColumnId = {};
|
|
|
|
for (const finding of findings) {
|
|
if (finding.kind === "query" || finding.kind === "definition") {
|
|
properties[finding.kind] += 1;
|
|
if (finding.kind === "query") {
|
|
queryByColumnId[finding.columnId] = (queryByColumnId[finding.columnId] ?? 0) + 1;
|
|
queryByFile.set(finding.file, (queryByFile.get(finding.file) ?? 0) + 1);
|
|
}
|
|
continue;
|
|
}
|
|
totals[finding.kind] += 1;
|
|
if (finding.kind === "column" && finding.traitFallback) traitFallbackCount += 1;
|
|
if (finding.kind === "deliberate") {
|
|
/*
|
|
FNXC:WorkflowLifecycleColumns 2026-07-30-10:00 (PR #2661 review — greptile P1, same class again):
|
|
Keyed by FILE **and COLUMN ID**, not a per-file integer. A per-file aggregate is offset within a
|
|
single file: remove one reviewed `todo` exemption, add a `in-review` one beside it, and the
|
|
number never moves — so a fresh guard hides inside an existing marker.
|
|
|
|
That is the third time this instrument has been defeated by an aggregate (repo total -> per file
|
|
-> per file per column). Each step narrows what can offset silently. The residual is a same-file
|
|
SAME-COLUMN swap, and that one is deliberate: two `todo` exemptions in one file are
|
|
interchangeable by definition, so there is nothing a reviewer could act on.
|
|
*/
|
|
const key = `${finding.file}\u0000${finding.columnId}`;
|
|
deliberateByFile.set(key, (deliberateByFile.get(key) ?? 0) + 1);
|
|
}
|
|
if (finding.kind !== "column") continue;
|
|
byColumnId[finding.columnId] = (byColumnId[finding.columnId] ?? 0) + 1;
|
|
byFile.set(finding.file, (byFile.get(finding.file) ?? 0) + 1);
|
|
}
|
|
|
|
return {
|
|
totals,
|
|
/* Advisory, deliberately OUTSIDE `totals`: see the note on `summarize`. */
|
|
traitFallbackCount,
|
|
byColumnId,
|
|
byFile: [...byFile].sort((a, b) => b[1] - a[1] || a[0].localeCompare(b[0])),
|
|
deliberateByFile: [...deliberateByFile].sort((a, b) => b[1] - a[1] || a[0].localeCompare(b[0])),
|
|
properties,
|
|
queryByColumnId,
|
|
queryByFile: [...queryByFile].sort((a, b) => b[1] - a[1] || a[0].localeCompare(b[0])),
|
|
};
|
|
}
|
|
|
|
/** Read + census a list of files. Callers own enumeration so this stays pure and testable. */
|
|
export function censusFiles(files, readFile = (f) => readFileSync(f, "utf8")) {
|
|
return files.flatMap((file) => findComparisons(file, readFile(file)));
|
|
}
|