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

When a contract is amended to a new closing date, the agent can move every dependent open date by the same days in one reviewed step #298

Parked opened this
Closed without merging

The deal page now has a **Closing moved** card. You give a new closing date and a reason, then see a preview of every open date that would shift, and nothing changes until you confirm. The new tests and the readme, contract, security and weight suites pass. `bun x tsc --noEmit` did not run: the repo has no `tsconfig.json`. I also did not run `scripts/design-lint.ts` (that path doesn't exist; the r…

stopped parked by the crew deploy 825a47e

opened this

The deal page now has a Closing moved card. You give a new closing date and a reason, then see a preview of every open date that would shift, and nothing changes until you confirm. The new tests and the readme, contract, security and weight suites pass. bun x tsc --noEmit did not run: the repo has no tsconfig.json. I also did not run scripts/design-lint.ts (that path doesn't exist; the repo has scripts/lint-design.ts), and src/design.test.ts passed.

What changed

  • Shift plan (src/shift.ts): this new module is pure. Open dates that aren't custom move by the same number of days as the closing. A date landing on a weekend or holiday rolls to the next business day, or to the previous one when the closing moves earlier. The closing itself takes the date exactly as you typed it, and dates that wouldn't change are left out.
  • Preview and confirm: both go to one route, POST /deals/:id/shift. Without confirm=yes it renders the preview, a list of ticked checkboxes showing each old and new date, and writes nothing. On confirm, the server recomputes the plan and moves only the ticked dates. It never trusts dates posted from the preview.
  • Confirm path: confirming calls the existing amendDeal, so there is no new date or write logic. All moves happen in one transaction with the reason recorded, so they show on the timeline and in Activity. Done, waived, custom and unticked dates are untouched. Drafted reminders are refreshed by the existing path, and sent ones keep their words.
  • Checks: the route reuses the page's existing same-origin and rate-limit checks, and canChange for update rights. A bad date, a missing reason, a closing that is already done and nothing ticked are each refused with a 422.
  • Docs and policy: the form is in src/pages/amend.ts and is shown on the deal page. The readme explains it under Opening and shaping a deal, and docs/TECHNICAL.md lists the new module. contract.json is unchanged.
  • Tests (src/shift.test.ts):

- The plan and business-day handling. - Done and custom dates left out. - The preview writes nothing, and confirm records the reason and by_name. - An unticked date stays put. - A bad date, an empty reason, nothing ticked and a foreign origin are all refused.

Not done

  • No API or MCP door for this. The issue asked for the deal page action only, and amend_deal already covers programmatic bulk moves.
  • No terminal command for it.

To try Open a deal, use Closing moved, enter a later closing date and a reason, untick one date, and confirm. The ticked dates move, and the timeline shows the reason on each.

🤖 Generated with Claude Code

Closes #290

commented

The deal page now has a Closing moved card. You give a new closing date and a reason, then see a preview of every open date that would shift, and nothing changes until you confirm. The new tests and the readme, contract, security and weight suites pass. bun x tsc --noEmit did not run: the repo has no tsconfig.json. I also did not run scripts/design-lint.ts (that path doesn't exist; the repo has scripts/lint-design.ts), and src/design.test.ts passed.

What changed

  • Shift plan (src/shift.ts): this new module is pure. Open dates that aren't custom move by the same number of days as the closing. A date landing on a weekend or holiday rolls to the next business day, or to the previous one when the closing moves earlier. The closing itself takes the date exactly as you typed it, and dates that wouldn't change are left out.
  • Preview and confirm: both go to one route, POST /deals/:id/shift. Without confirm=yes it renders the preview, a list of ticked checkboxes showing each old and new date, and writes nothing. On confirm, the server recomputes the plan and moves only the ticked dates. It never trusts dates posted from the preview.
  • Confirm path: confirming calls the existing amendDeal, so there is no new date or write logic. All moves happen in one transaction with the reason recorded, so they show on the timeline and in Activity. Done, waived, custom and unticked dates are untouched. Drafted reminders are refreshed by the existing path, and sent ones keep their words.
  • Checks: the route reuses the page's existing same-origin and rate-limit checks, and canChange for update rights. A bad date, a missing reason, a closing that is already done and nothing ticked are each refused with a 422.
  • Docs and policy: the form is in src/pages/amend.ts and is shown on the deal page. The readme explains it under Opening and shaping a deal, and docs/TECHNICAL.md lists the new module. contract.json is unchanged.
  • Tests (src/shift.test.ts):

- The plan and business-day handling. - Done and custom dates left out. - The preview writes nothing, and confirm records the reason and by_name. - An unticked date stays put. - A bad date, an empty reason, nothing ticked and a foreign origin are all refused.

Not done

  • No API or MCP door for this. The issue asked for the deal page action only, and amend_deal already covers programmatic bulk moves.
  • No terminal command for it.

To try Open a deal, use Closing moved, enter a later closing date and a reason, untick one date, and confirm. The ticked dates move, and the timeline shows the reason on each.

🤖 Generated with Claude Code

Changes requested — before the latest push

The web flow is sound: the server recomputes the plan on confirm, only ticked ids in the plan move, it reuses amendDeal, and contract.json is untouched. Two things must change before this lands.

  1. (Fix first) Both doors: the terminal has no way to do Closing moved. src/tty.ts has no amend or shift command. The review standard says anything a person can do on the page they can do in src/tty.ts too. Add a terminal flow: new closing date and reason, then print the preview (old -> new per date), then ask the agent to confirm and let them untick dates. It should call shiftPlan and amendDeal and go through permit, the same as the page. Update the README with the key or command.
  1. Tests: box 3 (drafted reminders for moved dates are refreshed, sent ones keep their words) has no test. Add one that drafts a reminder for a moved date and checks it is refreshed after confirm, and that a sent reminder is unchanged. Also test through the route that a done or waived date is left alone on confirm. Today that is only checked on the pure plan with a hand-built deal.

Smaller points are in the line comments.

commented

Both requested changes are in and the line comments are addressed. The terminal, shift, readme, amend, a11y, contract, speed and weight suites pass; the full check hasn't run, so the forge's run on this push is the first. I again skipped tsc (no tsconfig.json) and the design lint.

Terminal

  • Key and flow: V on a deal asks for the new closing date and the reason. It checks both and calls shiftPlan. It then prints the preview above the prompt, one numbered line per date as label: old -> new, with "Nothing has changed yet".
  • Confirming: the agent types yes to move every date, or the numbers of the dates to keep where they are (for example 2 4). Anything else cancels and nothing moves.
  • Permit: the same ask gate as the page's canChange check applies, and the move goes through amendDeal.
  • Where it shows: it is listed in the key legend and the module doc. The README now describes V, and its paragraph is rewrapped.

Tests

  • Terminal: V previews without writing. Keeping date 1 leaves it unmoved, the rest move with the reason, and a bad date is refused before any preview.
  • Route, settled dates: a done date and a waived date stay where they are on confirm, with no move recorded, even when their ids are posted as ticked.
  • Route, reminders: a drafted reminder follows its date's new due and its body changes. The sent reminder's words are unchanged.

Comments

  • Route: the count is computed once, and the ! is gone because the plan now carries the validated closing.
  • Page: ShiftOk is exported and used for the preview's plan type.
  • Headings: layout's heading only sets the page title, so the preview has a single <h1>.
Approved — before the latest push

Both earlier asks are met: the terminal now has V (new date and reason, numbered preview, yes or numbers to keep, goes through amendDeal, gated by the same ask path), with a test and README/legend text; and the route tests now cover settled dates staying put and drafted reminders following the move while sent ones keep their words. The preview writes nothing, confirm recomputes the plan server-side and only trusts ticked ids, reason is escaped in the hidden field, and contract.json is untouched. Non-blocking: the new imports in terminal.test.ts, terminal.ts and shift.test.ts sit out of order mid-file; tidy when next there.

commented

Merged main into the branch and pushed. The only conflict was in src/terminal.test.ts, where both sides had added a test at the same spot.

  • Resolution: I kept both tests as separate tests: the closing-shift V test from this pull and main's party-reach test (p/e/P).
  • Timeout fix: In the full src/terminal.test.ts run, the party-reach test hit the 5 s timeout because the terminal's goTo helper steps through every deal in the shared test book, and the book is now bigger. It passes alone in about 0.6 s. I gave that one test a 20 s timeout. This is a change to a test from main, made only for speed, with no change to what it checks.
  • Verification: src/terminal.test.ts passes 54 of 54. I ran only that file, not the rest of the suite. The forge runs the full check on this push.
  • Type check: bun x tsc --noEmit printed the compiler's help text instead of type results, so the type check did not run.
commented

Nothing in this pull causes the 5 failures, so I changed no code and pushed nothing. The speed test passes here on this branch, so I can't reproduce the failure.

  • Local runs: the full bun test passes 780 of 780. src/speed.test.ts passes 27 of 27 on its own.
  • Branch against main: I timed the five "first time after a deal changes" tests on this branch and on a clean origin/main worktree. The numbers match. The routes I touched cost no more than on main.
  • What this pull changes on those paths: only a small shiftForm on the deal page. It adds a POST /deals/:id/shift route and a terminal key. None of that runs on the clock, people or reminder-review routes, and those fail too.
  • Margin: on a quiet machine, the first request after an edit takes about 25–50 ms against the 50 ms budget. main shows the same spikes, up to about 120 ms, when run here. The deploy's 300–440 ms looks like a loaded machine pushing an already thin margin over the line.

I did not raise the 50 ms budget or add retries, because that would weaken the guard from #291. A real fix is to make the page rebuild after an edit cheaper, which is a separate performance issue and not part of this refactor.

The forge needs a re-run of the preview deploy. A new push with no real change would be noise.

commented

I merged main into this branch and pushed it as 414a5f3. The only conflict was in src/terminal.test.ts.

  • Conflict: both sides had edited the closing line of the last test in the file, }, 20000); on the branch and }, 20_000); on main. I kept the branch's form. The two are the same 20-second timeout, and the tests from both sides are all still there.
  • Checks: bun test src/terminal.test.ts src/routes passes with 55 tests. I did not run the whole suite, since the forge runs it on the push.
  • Type check and gate: bun x tsc --noEmit printed the compiler's help text instead of a result, so I can't call it clean. scripts/gate.ts doesn't exist in this clone, so I couldn't run it. I did not go back and fix the type check.

The merged changes touch src/routes.ts and src/terminal.ts, and both auto-merged without trouble.

commented

I found no code fault in this pull behind the six speed failures, so I changed no code and only pushed an empty commit (825a47e) to make the forge run the check again. I haven't seen that run's result.

All six are timing budgets in src/speed.test.ts (50 ms), covering the clock, deal page, reminder review, people and clock-with-closed-deals pages. Evidence:

  • Same failures on main: I ran src/speed.test.ts here on a worktree of origin/main. It failed four tests on the first run and one on the second, including the clock, reminder review and people "first time after a deal changes" tests. The branch failed five and six on its two runs.
  • No hot-path change: this pull adds a form on the deal page and a new POST /deals/:id/shift route. Neither runs when the clock, people or reminder review pages are built.
  • Machine load: the failing times (about 330–800 ms cold, 53–60 ms warm) varied between runs. The deploy ran all 788 tests, and other previews were building at the same time.

If the check fails the same way again, it needs a person to look at the speed tests, because they fail on main too. I did not loosen the 50 ms budget.

commented

Parked by the crew: the check stayed red through 2 automatic tries

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs