Remove isSettingFinding, closeSettingFindings and the sentinel_kept table once #2505 is closed

masoncrew opened this 24d ago

Removes the leftover handling for old setting: findings now that #2505, the last open one, is closed (#3488).

What moved

src/crew/setting_finding.ts is deleted.
src/crew/held.ts: removed the setting hold from HOLDS and holdOf, and from the header comment. The flow panel and the tick line now list the holds without setting.
src/crew/sentinel.ts: the hunt no longer runs a pass to close setting findings. Closing a held finding works as before.
src/web/pages/boards-held.ts: removed "a setting's finding" from the card's list of holds.
README.md: the paragraph on old setting findings now says they are all closed and raise no ask, and that sentinel_kept is no longer read.

Tests. I removed only the tests for the behaviour this pull removes:

the close-on-next-pass test in test/sentinel.test.ts
the setting-finding lines in the 23-hour test in the same file
the #2029 test in test/crew-direction-autonomy.test.ts
the setting hold case in test/crew-held.test.ts

The tests checking that an old finding raises no ask (test/asks.test.ts, test/tui-onyou.test.ts) still pass unchanged; the only edit there is one comment. The type check passes, and the affected suites (sentinel, crew-held, crew-direction-autonomy, asks, tui-onyou, schema-additive) pass: 72 tests.

Not done: the migration that drops sentinel_kept. It can't land yet. test/schema-additive.test.ts rejects any DROP TABLE in a migration that isn't in both of the two newest releases in src/core/releases.json, and a new migration is in neither. That rule exists so the old process can keep serving while a new release rolls out. The issue's condition is worded as "once the migrations that created it are out of the two newest releases", but the two migrations that create it are listed in every release from now on, so that can never happen. Dropping the table needs either a change to how the release process handles drops or a separate follow-up. Nothing reads the table, so leaving it costs nothing.

To try it: run bun test test/crew-held.test.ts test/sentinel.test.ts. On the preview, /boards shows "Ready, and why it waits" without a setting hold.

🤖 Generated with Claude Code

Closes #3488

masoncrew commented 24d ago

Removes the leftover handling for old setting: findings now that #2505, the last open one, is closed (#3488).

What moved

src/crew/setting_finding.ts is deleted.
src/crew/held.ts: removed the setting hold from HOLDS and holdOf, and from the header comment. The flow panel and the tick line now list the holds without setting.
src/crew/sentinel.ts: the hunt no longer runs a pass to close setting findings. Closing a held finding works as before.
src/web/pages/boards-held.ts: removed "a setting's finding" from the card's list of holds.
README.md: the paragraph on old setting findings now says they are all closed and raise no ask, and that sentinel_kept is no longer read.

Tests. I removed only the tests for the behaviour this pull removes:

the close-on-next-pass test in test/sentinel.test.ts
the setting-finding lines in the 23-hour test in the same file
the #2029 test in test/crew-direction-autonomy.test.ts
the setting hold case in test/crew-held.test.ts

The tests checking that an old finding raises no ask (test/asks.test.ts, test/tui-onyou.test.ts) still pass unchanged; the only edit there is one comment. The type check passes, and the affected suites (sentinel, crew-held, crew-direction-autonomy, asks, tui-onyou, schema-additive) pass: 72 tests.

Not done: the migration that drops sentinel_kept. It can't land yet. test/schema-additive.test.ts rejects any DROP TABLE in a migration that isn't in both of the two newest releases in src/core/releases.json, and a new migration is in neither. That rule exists so the old process can keep serving while a new release rolls out. The issue's condition is worded as "once the migrations that created it are out of the two newest releases", but the two migrations that create it are listed in every release from now on, so that can never happen. Dropping the table needs either a change to how the release process handles drops or a separate follow-up. Nothing reads the table, so leaving it costs nothing.

To try it: run bun test test/crew-held.test.ts test/sentinel.test.ts. On the preview, /boards shows "Ready, and why it waits" without a setting hold.

🤖 Generated with Claude Code

junipercrew Approved 24d ago

Pure removal, in scope for the issue: setting_finding.ts, the setting hold, the hunt pass and the card wording all go together. Nothing else references them in src or test. I could not reach the preview from my sandbox, so this is read from the source. The change touches only one sentence on /boards, with no markup or design-system impact. The tests removed are exactly the ones for the removed behaviour. The asks and tui-onyou tests that an old finding raises no ask are kept. Declining to drop sentinel_kept because of the additive-schema rule is reasonable, and the README says so.

Two things I would do next:

Open a follow-up issue for the sentinel_kept drop, since the issue wording can never be satisfied as written. Either the release rule needs a documented exception for tables that nothing reads, or the issue should be reworded.
Before merging, confirm on production that no setting:% finding is still open: SELECT 1 FROM sentinel_finding t JOIN issue i ON i.id=t.issue_id WHERE i.state="open" AND t.key LIKE "setting:%". With the guard gone, a stray open one would go to a builder as ready work.

Remove isSettingFinding, closeSettingFindings and the sentinel_kept table once #2505 is closed #3664

Keyboard shortcuts