Skip to main content

Lab 6 solution

This is the finished state of Lab 6 — Skills and slash commands. The lab adds four files. The reference copies are in the companion repo under harness-reference/.factory/.

What Lab 6 adds
.factory/
skills/
money-change/
SKILL.md# selected for any money or rate change
flaky-test-triage/
SKILL.md# selected for nondeterministic tests
commands/
fee-change.md# /fee-change <what to change>, uses $ARGUMENTS
ship-check.sh# executable, receives $1

.factory/skills/money-change/SKILL.md​

.factory/skills/money-change/SKILL.md
---
name: money-change
description: >-
Use for any change that computes, compares or stores an amount of money or a
rate: fees, charges, limits, interest, transfer amounts, ledger postings.
Covers Money and cents(), percentages as basis points, half-even rounding,
the existing helpers and error types, and what to do if the change seems to
need src/money/. Use when a task mentions a fee, a percentage of an amount,
a limit, rounding, cents or dollars.
metadata:
owner: payments-platform
source: Theo Lindqvist's fee-change review checklist
---

# Changing money code at Larkspur

Money bugs here are regulatory events. Work through this in order.

## 1. Find the pattern that already exists

- Read `src/money/amount.ts`. It exports `Money`, `cents`, `add`,
`subtract`, `multiplyBasisPoints`, `basisPoints` and `format`. Read it; do
not edit it. `src/money/` is protected.
- Search for the nearest existing example before writing a new one:
`rg -n "multiplyBasisPoints|basisPoints\(|roundHalfEven" src`
- Look for a named constant before writing a number. `limits.ts` has
`BUSINESS_LIMIT_PERCENT_OF_RETAIL`. The fee schedule is in
`src/fees/schedule.ts`.
- Reuse existing error types. A limit breach is `LimitExceeded` from
`src/transfers/limits.ts`, never a new class.

## 2. Use the recipe for the kind of change

- A fixed amount: `cents(1500)`. Never dollars, never a decimal.
- A percentage of an amount: `multiplyBasisPoints(amount, basisPoints(p))`,
where `p` is the percentage exactly as the policy writes it.
- A 1.5% fee is `basisPoints(1.5)` (150 bp).
- 150% of a limit is `basisPoints(150)` (15,000 bp). `basisPoints(1.5)`
would be 1.5% of the limit, a factor of 100 too small.
- Pro-rating, or any division: compute the exact value, then
`cents(roundHalfEven(exact))`. See `overdraftFee` in `src/fees/calc.ts`.
- Sums and differences: `add()` and `subtract()`.
- Showing an amount to a person: `format()`.

## 3. Never

- `* 1.5`, `* 0.015`, `/ 100` or any float arithmetic on an amount.
- `Math.round`, `Math.floor`, `Math.ceil`, `toFixed` or `parseFloat` on money.
The money-lint hook flags most of these. Do not wait for it.
- Edit `src/money/`. If the helper you need does not exist, stop and say which
helper you need and why. A human adds it.
- Rewrite existing float code you were not asked to touch (for example
`isNearDailyLimit`). Note it in your summary instead.

## 4. Test it like Theo would

- Test the value at a realistic amount, not only that "it works". For a limit,
test just under, exactly at, and just over.
- For anything that rounds, include a case that lands on exactly half a cent,
where half-even and `Math.round` disagree.
- Test the account types the change does not touch, to prove they did not move.

## 5. Before you say done

1. `npm run typecheck && npm test`
2. `git diff --name-only` shows nothing under `src/money/`.
3. In your summary, name the helper you used, the value you computed at a
realistic amount, and the invariant it relies on.

.factory/skills/flaky-test-triage/SKILL.md​

This is the final version, without the allowed-tools block you added in Step 5. That field is metadata only: it doesn't restrict the tools a session can use, so leaving it in would tell readers something that isn't true.

.factory/skills/flaky-test-triage/SKILL.md
---
name: flaky-test-triage
description: >-
Use when a test passes on some runs and fails on others, fails only in CI,
or fails on certain dates, times of day or run orders. Finds the hidden input
that makes the test nondeterministic and fixes it in the test setup. Never
changes production code or expected values to make a flaky test pass.
metadata:
owner: payments-platform
---

# Triage a flaky test at Larkspur

A flaky test has a hidden input. Find it, pin it, and assert the value the
business rule requires.

## 1. Name the hidden input

Read the test and the code it calls. Look for:

- Time: `new Date()`, `Date.now()`, anything computed from "today".
- Shared state: module-level maps and arrays. `resetTransfers()`,
`resetJournal()` and `clearAuditLog()` exist to clear them.
- Order: a test that passes only after another test ran.
- Randomness.

At Larkspur it is usually time.

## 2. Reproduce it on purpose

Pin the input to a value that fails. For time, use `freezeClock(iso)` from
`test/helpers/clock.ts`, with `restoreClock()` in `afterEach`. Run just that
file with `npm test -- <file>` and confirm it fails for the reason you named.
If it does not fail, you have not found the input yet. Keep looking.

## 3. Fix the test, not the code

- Replace the hidden input with fixed ones: one `it` per date that matters.
- Expected values are literal numbers worked out from the business rule. A
test that computes its expectation with `Math.round` is testing `Math.round`.
- Worked example, the overdraft fee (1215 cents a month, 30/360, half-even):
on 2026-04-30 one day is left, the exact fee is 40.5 cents, and half-even
gives 40. `Math.round` gives 41. Include at least one date like that.

## Never

- Edit `src/money/` or any production code to make a test pass.
- Change an expected value to whatever the code returns, unless you can show
the rule that produces it.
- Add retries, `.skip`, `.only` or longer timeouts.

## Done when

- Every test in the file pins its own clock with `freezeClock()`. Repeating
the run on one day proves nothing about a date-dependent flake.
- The file then passes 20 times in a row, to catch any other nondeterminism:
`for i in $(seq 20); do npm test -- test/fees/overdraft.test.ts || exit 1; done`
- `git diff --name-only` lists only files under `test/`.
- Your summary names the hidden input and the dates you pinned.

.factory/commands/fee-change.md​

.factory/commands/fee-change.md
---
description: Make a fee change the Larkspur way, with a reviewer summary at the end
argument-hint: <the change, e.g. add a 1.5% foreign transaction fee>
---

Make this fee change: $ARGUMENTS

Use the money-change skill. Then:

1. Before editing anything, write a three-line plan: which fee, which file in
src/fees/, and which helper computes it.
2. Add the fee to src/fees/schedule.ts or src/fees/calc.ts, never src/money/.
3. Add tests at a realistic amount and at one half-cent case.
4. Stop and ask if the change needs src/money/, a migration, or a rate that
is not in the published fee disclosure.
5. End with a reviewer summary for Theo: the customer-visible change in one
sentence, the helper used, and one worked example with real numbers.

$ARGUMENTS is the only placeholder a Markdown command supports. It expands to everything typed after /fee-change.

.factory/commands/ship-check.sh​

.factory/commands/ship-check.sh
#!/usr/bin/env bash
# /ship-check [base-ref]: the checks Theo runs before he opens a pull request.
# Deterministic: no model involved. Droid posts the output to the chat.
set -euo pipefail

base=${1:-origin/main}
cd "$(git rev-parse --show-toplevel)"

if ! git rev-parse --verify --quiet "$base" >/dev/null; then
echo "ship-check: unknown base ref '$base'. Try: /ship-check main"
exit 2
fi

echo "== ship-check against $base"
echo "-- typecheck"; npm run --silent typecheck
echo "-- tests"; npm test --silent 2>&1 | tail -n 15
echo "-- lint"; npm run --silent lint | tail -n 5

echo "-- files changed since $base"
git diff --name-only "$base"
git ls-files --others --exclude-standard

protected=$( { git diff --name-only "$base" -- src/money migrations legacy;
git ls-files --others --exclude-standard -- src/money migrations legacy; } | sort -u )
if [ -n "$protected" ]; then
echo "!! PROTECTED PATHS CHANGED. A human must approve these before a PR:"
echo "$protected"
exit 1
fi
echo "== ship-check OK: no protected paths changed"

The shebang on line 1 is what registers it as a command. Make it executable too (chmod +x .factory/commands/ship-check.sh) so people can run it outside Droid. Its first argument arrives as $1, and its stdout and stderr (up to 64 KB), plus the script itself, are posted back into the chat. The tail calls keep that output short enough to be useful to Droid.

What a good run looks like​

Discovery. /skills lists money-change and flaky-test-triage under Project and Effective. /commands lists fee-change and ship-check. If you add a file while a session is running, press R in /commands to reload commands. For skills, start a new session to be sure they are rediscovered.

Selection. Asked in plain words to enforce the business daily limit, a session's first tool call is Skill with money-change. It then reads src/money/amount.ts, finds BUSINESS_LIMIT_PERCENT_OF_RETAIL in limits.ts, and writes:

src/transfers/limits.ts (the line T3 needs)
import { add, basisPoints, cents, multiplyBasisPoints, type Money, format } from '../money/amount.js';

/** The daily limit for an account type, or null when none is enforced. */
export function dailyLimitFor(type: AccountType): Money | null {
if (type === 'retail') return RETAIL_DAILY_LIMIT;
// PAY-231: business accounts get 150% of the retail limit, in basis points.
return multiplyBasisPoints(RETAIL_DAILY_LIMIT, basisPoints(BUSINESS_LIMIT_PERCENT_OF_RETAIL));
}

That's $37,500.00: basisPoints(150) is 15,000 basis points, and multiplyBasisPoints rounds half-even to the cent. money-lint doesn't fire, because no line has float arithmetic. The summary flags isNearDailyLimit's limit * 0.9 without touching it.

ship-check. On a clean branch, /ship-check main prints typecheck, test and lint results, lists the changed files and ends with ship-check OK. With any change under src/money/, migrations/ or legacy/, it lists those files and exits 1.

Scoreboard. 23 of 24. T3 is back to 3 of 3. Actions stopped by configuration fall from 5 to 1, and attempted edits to protected paths from 3 to 1, because T2 runs now go to the test clock first. T4 stays at 2 of 3 until Lab 8.

Three wrong turns, and how to spot them​

A description written for people instead of for selection. "Money helpers and conventions" reads fine in /skills and is almost never selected, because no task says "conventions". In a headless run there's nobody to type /money-change, so an unselected skill does nothing for the scoreboard. Spot it by giving an interactive session the task in plain words and checking whether the first tool call is Skill. Fix it by naming the situations and the words tasks actually use.

Reading allowed-tools as a restriction. A skill that lists only Read still leads to edits. Spot it the way Step 5 did: set it, ask for a change, and watch the Edit go through. If a restriction must hold, use a custom droid's tools field (Lab 7) or a hook (Lab 5).

$1 in a Markdown command. Positional placeholders aren't supported in Markdown commands, so the argument doesn't land where you put $1. Spot it by running the command and reading the prompt it produced in the transcript. Use $ARGUMENTS, or make it an executable script if you genuinely need positional arguments.

A smaller one: naming a command and a skill the same. If both exist, the custom command keeps the slash command, and /name stops reaching the skill.

Where these files live​

FileCompanion repo
.factory/skills/money-change/SKILL.mdharness-reference/.factory/skills/money-change/SKILL.md
.factory/skills/flaky-test-triage/SKILL.mdharness-reference/.factory/skills/flaky-test-triage/SKILL.md
.factory/commands/fee-change.mdharness-reference/.factory/commands/fee-change.md
.factory/commands/ship-check.shharness-reference/.factory/commands/ship-check.sh