fix(glasses): settings silently discarded the operator's own lane names, so notifications never fired (#2973)

## The operator configures their lanes, and the plugin throws it away

`TaskColumn` was the closed legacy union and `COLUMN_SET` gated every
settings read against it:

```ts
.filter((value): value is TaskColumn => COLUMN_SET.has(value as TaskColumn));
```

**`notifyOnColumns` is a free-text string array in the schema** (`type:
"array", itemType: "string"`) — the UI invites any lane name. So an
operator on a renamed board types `checking`:

1. `getNotifyColumns` filters it out — not in the legacy five
2. `columns.length === 0`, so it substitutes `DEFAULT_NOTIFY_COLUMNS` =
`["in-review"]`
3. their board has no `in-review`
4. `notifier.ts` builds `new Set(getNotifyColumns(...))`, and
`diffSnapshots` tests `notifyOnColumns.has(task.column)`

**No notification ever fires.** The feature is silently off while the
setting reads as configured, and nothing surfaces an error.

`quickCaptureDefaultColumn` had the same shape with an extra irony: it
was rewritten to `todo` *before* `normalizeCaptureColumn` saw it — and
that function already validates against the board's **declared** columns
and falls back to the workflow's own intake lane. The pre-filter
destroyed the operator's answer immediately before the code that could
have honoured it.

## Fix

Validation is now **structural** (non-empty string) rather than
**vocabulary-based**. `TaskColumn` mirrors core's `ColumnId`
(`LegacyTaskColumn | (string & {})`), so legacy ids keep autocomplete
while custom ids are admitted. `COLUMN_SET` survives only as the
quick-capture dropdown's suggestions, not as a gate.

**Deliberately not resolving the board's columns.** That needs a
board-columns endpoint `FusionApiClient` does not have — a new read, not
a rename — and that gap is already recorded in this file by an earlier
audit. This change is orthogonal to it: accepting operator input
requires no resolution at all, which is why it doesn't wait on the
endpoint.

**Trade-off, taken knowingly:** a typo'd lane is now honoured and will
match no card. That is the milder failure. Before, a *correct* custom
lane was discarded categorically, so renamed boards could not use the
feature at all; now the only broken case is one the operator typed wrong
and can see in their own settings.

## Two existing assertions asserted the defect

```ts
expect(getNotifyColumns({ notifyOnColumns: ["nope"] })).toEqual(["in-review"]);
expect(getQuickCaptureColumn({ quickCaptureDefaultColumn: "bad-column" })).toBe("todo");
```

I checked provenance before touching them: they date to the plugin's
original commits (FN-3738 / FN-3970), when the five ids genuinely were
the whole vocabulary. They carry no reasoning comment and no later
change defended custom-lane rejection as a contract — so this is a stale
assumption being corrected, not a peer's tested decision being
overwritten. Structural rejection (non-strings, blanks, whitespace-only)
and trimming are still asserted, because that part was always right.

## Revert proof

```
AssertionError: expected [ 'in-review' ] to deeply equal [ 'checking', 'shipped' ]
AssertionError: expected [ 'todo' ] to deeply equal [ 'todo', 'spaced' ]
      Tests  2 failed | 5 passed (7)
```

## Verification (measured)

- plugin suite — **196 passed / 19 files**
- `tsc --noEmit`, `eslint` — clean
- `lifecycle-column-census --strict`, `check-fnxc-future-dates` — green

No changeset: the plugin is `private: true` and is not bundled into the
published CLI.
This commit is contained in:
gsxdsm
2026-07-30 22:30:38 -07:00
committed by GitHub
parent 2fd798cb36
commit eb9cd431c1
2 changed files with 68 additions and 10 deletions

View File

@@ -42,14 +42,38 @@ describe("settings accessors", () => {
expect(getPollingIntervalMs({ pollingIntervalSeconds: Number.NaN })).toBe(30000);
});
it("filters notify columns and falls back when invalid", () => {
expect(getNotifyColumns({ notifyOnColumns: ["todo", " nope ", "in-review", 4] })).toEqual(["todo", "in-review"]);
expect(getNotifyColumns({ notifyOnColumns: ["nope"] })).toEqual(["in-review"]);
/*
FNXC:PluginLifecycleColumns 2026-07-30-23:40:
THE INVARIANT: a lane name the operator typed is theirs to choose, not ours to veto.
These two cases previously asserted the opposite — that a column outside the legacy five is
"invalid" and gets replaced. That assumption dates to the plugin's original commits (FN-3738 /
FN-3970), when the five ids WERE the whole vocabulary; it is not a deliberate contract anyone
defended later, and it is now the defect: `notifyOnColumns` is a free-text array, so an operator on
a renamed board types `checking`, it is filtered to nothing, and the fallback `in-review` matches no
card — notifications silently never fire while the setting looks configured.
Structural rejection is still asserted, because that part was always right: non-strings, blanks and
whitespace-only entries are not lane names. What is gone is rejection on VOCABULARY.
Reverted, the first two expectations below fail — `["checking","shipped"]` comes back as
`["in-review"]`, and `backlog` comes back as `todo`.
*/
it("keeps the operator's own lane names instead of substituting a legacy fallback", () => {
expect(getNotifyColumns({ notifyOnColumns: ["checking", "shipped"] })).toEqual(["checking", "shipped"]);
expect(getQuickCaptureColumn({ quickCaptureDefaultColumn: "backlog" })).toBe("backlog");
});
it("validates quick capture column", () => {
it("still rejects entries that are not lane names at all, and still trims", () => {
expect(getNotifyColumns({ notifyOnColumns: ["todo", " spaced ", "", " ", 4] })).toEqual(["todo", "spaced"]);
expect(getNotifyColumns({ notifyOnColumns: [] })).toEqual(["in-review"]);
expect(getNotifyColumns({ notifyOnColumns: "not-an-array" })).toEqual(["in-review"]);
expect(getQuickCaptureColumn({ quickCaptureDefaultColumn: " " })).toBe("todo");
});
it("still accepts the legacy ids", () => {
expect(getNotifyColumns({ notifyOnColumns: ["todo", "in-review"] })).toEqual(["todo", "in-review"]);
expect(getQuickCaptureColumn({ quickCaptureDefaultColumn: "done" })).toBe("done");
expect(getQuickCaptureColumn({ quickCaptureDefaultColumn: "bad-column" })).toBe("todo");
});
it("respects explicit boolean for agent actions", () => {

View File

@@ -26,9 +26,43 @@ bug and it is unchanged here; I am not papering over it with a guess.
*/
const DEFAULT_QUICK_CAPTURE_COLUMN = "todo";
type TaskColumn = "triage" | "todo" | "in-progress" | "in-review" | "done";
/*
FNXC:PluginLifecycleColumns 2026-07-30-23:40:
The operator's OWN lane names are accepted, instead of being silently discarded against a legacy enum.
const COLUMN_SET = new Set<TaskColumn>(["triage", "todo", "in-progress", "in-review", "done"]);
`TaskColumn` was the closed legacy union and `COLUMN_SET` gated every settings read against it. The
consequence was not a wrong label, it was the operator's configuration being thrown away in silence:
- `notifyOnColumns` is a FREE-TEXT string array in the schema below, so an operator on a renamed
board types `checking` — and `getNotifyColumns` filtered it out, hit `columns.length === 0`, and
substituted `DEFAULT_NOTIFY_COLUMNS` (`in-review`), a lane their board does not have. The notifier
tests `notifyOnColumns.has(task.column)`, so NO notification ever fires. The feature is off, the
setting looks set, and nothing reports an error.
- `quickCaptureDefaultColumn` was rewritten to `todo` before `normalizeCaptureColumn` ever saw it —
which is the smarter path, because it already checks the value against the board's DECLARED
columns and falls back to the workflow's own intake lane. The pre-filter destroyed the operator's
answer just before the code that could have honoured it.
Widened to mirror core's `ColumnId` (`Column | (string & {})`): the legacy ids keep autocomplete, and
a custom id is admitted. Validation is now "a non-empty string", because a plugin reaching Fusion over
HTTP cannot know the board's vocabulary and the operator can — second-guessing them against five
hardcoded ids is what broke this.
Deliberately NOT resolving the board's columns here. That needs a board-columns endpoint
`FusionApiClient` does not have (a new read, not a rename), and that gap is recorded above; this
change is orthogonal to it — accepting operator input requires no resolution at all.
Trade-off, taken knowingly: a typo'd lane is now honoured rather than rejected, and it will match no
card. That is the milder failure. Before, a CORRECT custom lane was discarded categorically, so
renamed boards could not use the feature at all; now the only broken case is one the operator typed
wrong and can see in their settings.
`COLUMN_SET` survives ONLY as the enum suggestions for the quick-capture dropdown, not as a gate.
*/
type LegacyTaskColumn = "triage" | "todo" | "in-progress" | "in-review" | "done";
type TaskColumn = LegacyTaskColumn | (string & {});
const COLUMN_SET = new Set<LegacyTaskColumn>(["triage", "todo", "in-progress", "in-review", "done"]);
export const settingsSchema: Record<string, PluginSettingSchema> = {
fusionApiBaseUrl: {
@@ -110,13 +144,13 @@ export function getNotifyColumns(settings: Record<string, unknown>): TaskColumn[
const columns = raw
.filter((value): value is string => typeof value === "string")
.map((value) => value.trim())
.filter((value): value is TaskColumn => COLUMN_SET.has(value as TaskColumn));
.filter((value): value is TaskColumn => value.length > 0);
return columns.length > 0 ? columns : ([...DEFAULT_NOTIFY_COLUMNS] as TaskColumn[]);
}
export function getQuickCaptureColumn(settings: Record<string, unknown>): TaskColumn {
const raw = getSettingString(settings, "quickCaptureDefaultColumn");
return raw && COLUMN_SET.has(raw as TaskColumn) ? (raw as TaskColumn) : DEFAULT_QUICK_CAPTURE_COLUMN;
/* `getSettingString` already trims and rejects blank, so a surviving value is a real lane name. */
return getSettingString(settings, "quickCaptureDefaultColumn") ?? DEFAULT_QUICK_CAPTURE_COLUMN;
}
export function agentActionsEnabled(settings: Record<string, unknown>): boolean {