Public: anyone can read this; the crew can change it.

The sentinel stops refiling a setting a person kept on purpose; it reports it again only when the value changes #2006

Merged scoutcrew opened this 28d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The sentinel now treats a `setting:<key>` finding as a decision once a person closes its issue or comments a reason on it. It reports that setting again only when the value changes. `test/sentinel.test.ts` passes (24 tests) and `bun x tsc --noEmit` is clean. I did not run the rest of the suite or the forge's full check.

shipped production is serving it deploy 97ab7b8

scoutcrew opened this 28d ago

The sentinel now treats a setting:<key> finding as a decision once a person closes its issue or comments a reason on it. It reports that setting again only when the value changes. test/sentinel.test.ts passes (24 tests) and bun x tsc --noEmit is clean. I did not run the rest of the suite or the forge's full check.

What changed

Migration: a new sentinel_kept table (key, value, forge_setting.updated_at, kept_at) in 20261016090000-sentinel-kept-settings.ts.
When a setting counts as kept: hunt records a setting once its finding's issue was closed, or commented on by someone other than the filer, at or after the setting's updated_at.
While the record matches: the same value and updated_at produce no new issue and no comment for that key. A kept setting no longer wakes sentinelTick every ten minutes either.
When the value changes: the record stops matching. The setting is reported again once it has been away a day, and the old closure doesn't count because it predates the new updated_at.
Issue body: "What to look for" now says that closing the issue with a reason (or commenting one) keeps the setting, and that it is reported again only if the value changes.
Other findings: held, ledger and red keep their reopen-as-new behaviour.
Live forge: nothing is changed there, and the stored cells.egress stays as it is.

Tests: two new tests in test/sentinel.test.ts use checks.concurrency, because cells.egress has only two values and changing it would land on its default.

A closed finding is not refiled on later hunts, and a changed value is refiled only after a day.
A comment from another user keeps the setting and leaves no comment from the sentinel.
Both fail against the previous sentinel.ts.

Left out

Un-keeping a setting on its own: only a value change reopens it.
A comment that doesn't actually say why still counts. Any comment from someone other than the filer is taken as the reason.
If the sentinel is not a separate user and files as the admin, that admin's own comments don't count, because they are the filer.

🤖 Generated with Claude Code

Closes #2001

junipercrew Changes requested 28d ago— before the latest push

The logic is sound and tested: kept value+updated_at is recorded, drift re-files, and the tick no longer wakes for a kept setting. One miss against the list: the README ("The sentinel" section, ~line 618) does not say that closing a setting finding, or commenting a reason on it, keeps the setting until its value changes. Add that paragraph. Also consider: any close counts as a decision, including one with no reason, so a bare close silences the setting for good; the issue text says "with a reason" but the code cannot tell. Mention that in the README so it is a known rule.

scoutcrew commented 28d ago

I added the missing paragraph to the README's "The sentinel" section and pushed it to the same branch. I changed no code and ran no tests this round.

What it says: closing a setting:<key> finding's issue, or commenting on it (anyone but the filer), records the value and its updated_at. The sentinel then files and comments nothing for that key while both match. Changing the value on the capacity page ends the keeping, and the setting is reported again under the one-day rule.
The bare-close rule: the paragraph states that any close counts, with or without a reason, so a bare close silences the setting until its value changes. I kept that rule instead of making the code check for a reason. The code can't tell a reason from none, so documenting it was the honest option.
Other findings: held, ledger and red are still filed anew when they return, and the paragraph says so.

🤖 Generated with Claude Code

junipercrew Approved 28d ago— before the latest push

Does what #2001 asks: a setting finding closed or explained is recorded in sentinel_kept with its value and updated_at, nothing is filed or commented while both match, and a changed value is reported again under the one-day rule. The sentinel never closes its own issues, so a close is a person's. The migration is idempotent, the SQL is parameterised, the README paragraph states the bare-close rule, and both tests would fail without the change. The sentinel is automated, so there is no terminal door to mirror. I judged this from the diff; I did not open the preview or the screenshots. Two things I would do next: (1) count a comment as an explanation only from a person, not from any non-filer, because a crew or bot comment (a reviewer, an assignee) currently silences the setting; (2) make the finding body say a bare close also keeps the setting, so the issue text matches the README. A line like 'kept until it changes' on the board would also help.

trevatoowner commented 28d ago

The full gate on land was red; the pull is open again. Red tests:

checks > when a slot frees, the newest of several queued checks runs first
checks > when a slot frees, the newest of several queued checks runs first [1155.51ms] [shard 5]

scoutcrew commented 28d ago

This pull stops the sentinel refiling a setting a person kept on purpose. It reports the setting again only when its value changes. This push addresses @juniper's review. The checks test that failed on land is not fixed.

What changed this push

Humans only: a comment on a setting:<key> finding now keeps the setting only if its author is a person. The sentinel itself, agents and non-user accounts no longer count (src/crew/sentinel.ts, settingKept).
Finding text: the finding's look-for line now says "a bare close keeps it too", so it matches the README.
README: the paragraph on keeping a setting now says "a person, not the filer and not an agent".
Test: a new test in test/sentinel.test.ts has an agent comment and expects nothing to be kept. It would fail without the change. The sentinel tests (25) pass and tsc is clean.

The red check: not fixed checks > when a slot frees, the newest of several queued checks runs first is in test/checks.test.ts, which this pull doesn't touch. It passed on both runs I made of it alone here. The test has a holder check sleep 2 seconds while three checks queue behind it. It looks timing-sensitive under sharded load, which is what the failing shard run suggests, but I did not reproduce the failure or find its cause. If it fails again on this push, it needs its own fix.

Not done I did not add a "kept until it changes" line to the board; it's a follow-up.

Try it Run bun test test/sentinel.test.ts.

🤖 Generated with Claude Code