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

Estimates are recorded against an issue, start from the forge's own history,… #11983

Merged opened this in answer to

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

Merged

Estimates now cover the three problems from the steward's first real use. The changes are pushed. `tsc`, `scripts/gate.ts`, `design-lint` and `test/estimates*.test.ts` pass. 546 of 547 in a wider sample of 57 related test files pass. The one failure is `test/guest-contribute.test.ts`, where `git push` over HTTP returns 128 in this sandbox. I did not touch that path. I did not run the whole suite,…

shipped production is serving it deploy 1cfdf1a

opened this

Estimates now cover the three problems from the steward's first real use. The changes are pushed. tsc, scripts/gate.ts, design-lint and test/estimates*.test.ts pass. 546 of 547 in a wider sample of 57 related test files pass. The one failure is test/guest-contribute.test.ts, where git push over HTTP returns 128 in this sandbox. I did not touch that path. I did not run the whole suite, and scripts/fast.ts said the diff was too broad to name tests.

What changed

  • By-hand releases: at boot, a forge whose FORGE_VERSION/FORGE_COMMIT is newer than its newest served release records itself in keeper_release. The row has the version, the commit, went-live equal to the boot time, and how = "by hand". It then resolves open live estimates. The code is src/keeper/by_hand.ts, called from main.ts. It does nothing for an unstamped forge, a preview, an older version, or a version already recorded.
  • done event: an estimate can now mean "finished". It resolves when the issue closed as done has stayed closed 24 h, or when a release carrying its last merged pull went live, whichever came first. Time passing is no event, so it also settles on a five-minute timer. It is accepted by the API, MCP, terminal and docs/features/estimates.md, and named in the crew brief.
  • Refusal wording: the "already happened" refusal now names the event and lists the events that can still be estimated, for example "merged has already happened… still: live, done."
  • Reopen rate: the basis line now ends with "reopened after closing: 3 of 8", and base.reopened carries {n, of}. It counts issues a pull closed that were reopened later, including ones that are open again. Those count only toward the rate, not toward any duration.
  • Migrations: two new ones. One rebuilds issue_estimate to widen its event check, because SQLite cannot widen it in place. The other adds keeper_release.how. Both are pinned.

What I chose not to do

  • No back-fill: the eight earlier hand releases (0.7.552 to 0.7.559) are not recreated, because nothing records when each went live. A merge made before the first recorded release resolves live at that release's boot time, which overstates those waits.
  • Scorecard rows: I did not enter the #11953 data points. They were recorded by hand and the forge never records estimates by hand, so a person should add them.
  • "Last linked pull": I took it to mean the newest merged pull whose description closes the issue, which is what the forge already reads.

How to try it

  • Ask estimate on any issue: the basis ends with the reopen count.
  • Record event: done on an issue that has already merged a pull.
  • Run a forge with FORGE_VERSION=0.7.561 FORGE_COMMIT=<sha> and check /changelog, or the keeper_release row with how = "by hand".
  • The new tests are in test/estimates_done.test.ts.

Closes #11955

commented

Estimates now cover the three problems from the steward's first real use. The changes are pushed. tsc, scripts/gate.ts, design-lint and test/estimates*.test.ts pass. 546 of 547 in a wider sample of 57 related test files pass. The one failure is test/guest-contribute.test.ts, where git push over HTTP returns 128 in this sandbox. I did not touch that path. I did not run the whole suite, and scripts/fast.ts said the diff was too broad to name tests.

What changed

  • By-hand releases: at boot, a forge whose FORGE_VERSION/FORGE_COMMIT is newer than its newest served release records itself in keeper_release. The row has the version, the commit, went-live equal to the boot time, and how = "by hand". It then resolves open live estimates. The code is src/keeper/by_hand.ts, called from main.ts. It does nothing for an unstamped forge, a preview, an older version, or a version already recorded.
  • done event: an estimate can now mean "finished". It resolves when the issue closed as done has stayed closed 24 h, or when a release carrying its last merged pull went live, whichever came first. Time passing is no event, so it also settles on a five-minute timer. It is accepted by the API, MCP, terminal and docs/features/estimates.md, and named in the crew brief.
  • Refusal wording: the "already happened" refusal now names the event and lists the events that can still be estimated, for example "merged has already happened… still: live, done."
  • Reopen rate: the basis line now ends with "reopened after closing: 3 of 8", and base.reopened carries {n, of}. It counts issues a pull closed that were reopened later, including ones that are open again. Those count only toward the rate, not toward any duration.
  • Migrations: two new ones. One rebuilds issue_estimate to widen its event check, because SQLite cannot widen it in place. The other adds keeper_release.how. Both are pinned.

What I chose not to do

  • No back-fill: the eight earlier hand releases (0.7.552 to 0.7.559) are not recreated, because nothing records when each went live. A merge made before the first recorded release resolves live at that release's boot time, which overstates those waits.
  • Scorecard rows: I did not enter the #11953 data points. They were recorded by hand and the forge never records estimates by hand, so a person should add them.
  • "Last linked pull": I took it to mean the newest merged pull whose description closes the issue, which is what the forge already reads.

How to try it

  • Ask estimate on any issue: the basis ends with the reopen count.
  • Record event: done on an issue that has already merged a pull.
  • Run a forge with FORGE_VERSION=0.7.561 FORGE_COMMIT=<sha> and check /changelog, or the keeper_release row with how = "by hand".
  • The new tests are in test/estimates_done.test.ts.
commented

I merged main into the branch and pushed it (51b32d9e9). The pull should merge cleanly again.

The conflicts came from main already carrying an earlier merge of this issue's work (#11964). Its estimates files were identical to this branch's state before my last commit. I kept this branch's version of all of them (the done event, the by-hand release record, the reopen rate and the clearer refusal). In src/crew/brief.ts I kept the wording that names done.

main also brought a new rule that comment lines wrap at 100 columns, and eight of my comment lines broke it. I rewrapped them, including the comment in the done migration. Because that migration isn't on the base yet, its pin was still valid, and pin-migrations reported nothing to re-pin.

tsc and scripts/gate.ts are clean. The estimates tests, plus the two tests main brought in (ssh-tell-first-ask and traffic-truth), pass: 35 of 35. I did not run the full suite.

commented

Three of the seven failures came from this pull and are fixed and pushed. The other four fail identically on a clean checkout of origin/main, so I left them alone.

What changed

  • test/schema-additive.test.ts: the estimate-done migration is now listed as a rebuild. It copies issue_estimate to widen its event CHECK to admit done, which SQLite can only do by dropping and renaming the table. The columns and rows are unchanged and the previous release never reads or writes a done row, so it is safe across a roll.
  • test/job-slices.test.ts: the new estimates-due job (a five-minute timer that settles due done estimates) is named as untimed, with the reason. The fixture records no estimates.
  • docs/MCP.md: regenerated with bun scripts/mcp-table.ts because the estimate tool's row changed.

Checked

  • The migration, MCP table, job-slices and estimates-done tests pass: 24 pass, 0 fail.
  • scripts/gate.ts and tsc --noEmit are clean.
  • I merged the latest origin/main first.

Not mine

  • These four tests fail on a clean origin/main worktree: timers-named, flow-clock, unused-exports and stall-scans. The causes are src/runs/log.ts, src/spawn/stopped.ts, src/flow/pipeline.ts, and the exports mergedPullClosingScan, pinCrawlLag and bufferRunLogs.
  • None of those files are in this pull. They need a fix on main, so the forge check will likely still show those four failing.

Try it: open the preview at https://lab--11983--forge-trevato-apps.open-platform.sh and record an estimate with event done on an issue.

Approved

Read the diff against the security, terminal, tests, docs and cleanliness list. No findings. All writes still go through recordEstimate with its existing auth checks, and the new queries are parameterized. The terminal screen and the MCP, API and brief text all name done, and the how-to is in docs/features/estimates.md, not README. The migrations are pinned and listed in schema-additive, and every new behaviour has a test. Not exercised: I did not curl the preview or open the screenshots. Two small notes for later, neither blocking: the if (!shape.ok) assertion in test/estimates_done.test.ts is conditional and would pass vacuously, and settleDone compares ISO strings lexically, which holds only while all stored instants share the toISOString format.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs