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

Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:2aw0b8usmp is past its budget: trevato/forge: "a repository name with markup in it is escaped in the select, never raw" failed on 3 pulls in the last … #10575

Merged opened this

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

Merged

This makes the sentinel's test, "a repository name with markup in it is escaped in the select, never raw", depend only on the code under test. It also fixes a test-timing race that kept the last attempt red (#10523's check).

shipped production is serving it deploy f21b126

opened this

This makes the sentinel's test, "a repository name with markup in it is escaped in the select, never raw", depend only on the code under test. It also fixes a test-timing race that kept the last attempt red (#10523's check).

The sentinel's test (test/feed-composer-language.test.ts, already on the branch from the earlier tries): the test checked for a bare x">. That string also matches data-draft="<user id>" whenever alice's random id happens to end in x, so it failed sometimes with nothing wrong in the code. It now checks for alice/x", which only the raw repository name can produce. The positive check, that the escaped name is in the page, is unchanged.

Why #10523 stayed red: three other tests failed on it:

  • test/crew-loop.test.ts, "the third changes-requested hands the pull to a person" — fixed in this push. A review counts as news to the builder only when its timestamp is later than the start of the builder's last run (newsSince in src/crew/news.ts, strict > to the millisecond). The test marks the follow-up run as started and posts the next verdict straight away. The run's start comes from SQLite's clock and the review's time from JS, so the two can fall in the same millisecond, or the start can be one millisecond ahead. Round two then reads as "nothing new" and no follow-up is queued: Expected: 2, Received: 1. I reproduced that exact failure by putting the run's start 1 ms ahead. The test's verdict helper now waits until the clock is past the newest run's start before posting, and with the 1 ms skew still in place the tests pass. Only the test changed: in real use review rounds are minutes apart, so the strict comparison in news.ts stays.
  • The two real-child-forge cell tests (test/spawn-cell-e2e.test.ts, test/spawn-cell.test.ts) — not touched. They need bwrap, socat and unix sockets, so they are skipped in this sandbox and I could not run them. The check log was cut before their shards (8 and 18), so I couldn't diagnose them either. The check machine had about 16% disk free at the time, and nothing in this branch touches booting a cell. If they fail again on this push, they need their own issue.

Also on the branch from the earlier tries:

  • src/crew/sandbox.ts: when the data directory is inside the machine's temp directory, an agent's shell is still denied the other runs' fr- temp directories.
  • test/child-quota.test.ts: the memory limit leaves room for a second 2 GiB small child.
  • scripts/unreached-allow.json: drops the stale src/kernel/why_now.ts entry.

Checks run here: bun scripts/gate.ts and bun x tsc --noEmit pass, and bun test test/crew-loop.test.ts test/feed-composer-language.test.ts test/crew-sandbox.test.ts test/child-quota.test.ts passes (57 tests). I didn't run the full suite; the forge runs it on the push.

🤖 Generated with Claude Code

Closes #10486

commented

This makes the sentinel's test, "a repository name with markup in it is escaped in the select, never raw", depend only on the code under test. It also fixes a test-timing race that kept the last attempt red (#10523's check).

The sentinel's test (test/feed-composer-language.test.ts, already on the branch from the earlier tries): the test checked for a bare x">. That string also matches data-draft="<user id>" whenever alice's random id happens to end in x, so it failed sometimes with nothing wrong in the code. It now checks for alice/x", which only the raw repository name can produce. The positive check, that the escaped name is in the page, is unchanged.

Why #10523 stayed red: three other tests failed on it:

  • test/crew-loop.test.ts, "the third changes-requested hands the pull to a person" — fixed in this push. A review counts as news to the builder only when its timestamp is later than the start of the builder's last run (newsSince in src/crew/news.ts, strict > to the millisecond). The test marks the follow-up run as started and posts the next verdict straight away. The run's start comes from SQLite's clock and the review's time from JS, so the two can fall in the same millisecond, or the start can be one millisecond ahead. Round two then reads as "nothing new" and no follow-up is queued: Expected: 2, Received: 1. I reproduced that exact failure by putting the run's start 1 ms ahead. The test's verdict helper now waits until the clock is past the newest run's start before posting, and with the 1 ms skew still in place the tests pass. Only the test changed: in real use review rounds are minutes apart, so the strict comparison in news.ts stays.
  • The two real-child-forge cell tests (test/spawn-cell-e2e.test.ts, test/spawn-cell.test.ts) — not touched. They need bwrap, socat and unix sockets, so they are skipped in this sandbox and I could not run them. The check log was cut before their shards (8 and 18), so I couldn't diagnose them either. The check machine had about 16% disk free at the time, and nothing in this branch touches booting a cell. If they fail again on this push, they need their own issue.

Also on the branch from the earlier tries:

  • src/crew/sandbox.ts: when the data directory is inside the machine's temp directory, an agent's shell is still denied the other runs' fr- temp directories.
  • test/child-quota.test.ts: the memory limit leaves room for a second 2 GiB small child.
  • scripts/unreached-allow.json: drops the stale src/kernel/why_now.ts entry.

Checks run here: bun scripts/gate.ts and bun x tsc --noEmit pass, and bun test test/crew-loop.test.ts test/feed-composer-language.test.ts test/crew-sandbox.test.ts test/child-quota.test.ts passes (57 tests). I didn't run the full suite; the forge runs it on the push.

🤖 Generated with Claude Code

commented

This fixes the sentinel's test, "a repository name with markup in it is escaped in the select, never raw", by making its result depend only on the code under test. It also fixes a timing race in the crew-loop tests that kept the previous try (#10523) red.

The sentinel's test (test/feed-composer-language.test.ts): it checked for a bare x">. That string also matches data-draft="<user id>" whenever alice's random user id ends in x, so the test sometimes failed with nothing wrong. It now checks for alice/x", which only the raw repository name can produce. The check that the escaped name is on the page is unchanged.

The crew-loop race (test/crew-loop.test.ts): a review only counts as news to the builder if it is written after the builder's last run started, compared to the millisecond (newsSince in src/crew/news.ts). The test marks the follow-up run as started and posts the next verdict straight away. The run's start time comes from SQLite's clock and the review's from JavaScript's, so the two can land in the same millisecond, or the start can be a millisecond ahead. The round then reads as "nothing new" and no follow-up is queued, which is the Expected: 2, Received: 1 failure on #10523's check. I reproduced it by setting the run's start 1 ms ahead. The verdict helper now waits until the clock has passed the newest run's start, and with that 1 ms offset still in place the tests passed. Only the test changed: real review rounds are minutes apart.

Merge with main: main fixed the run temp directory sitting beside the data directory itself, in #10551. I took main's version of src/crew/sandbox.ts, test/child-quota.test.ts and the conflicting part of test/crew-sandbox.test.ts. On top of main, the branch keeps only a few extra assertions in test/crew-sandbox.test.ts, for a child forge's run:

  • another run's fr- temp directory is denied;
  • the agent's own working directory is not.

Not run here: the two real-child-forge cell tests (test/spawn-cell-e2e.test.ts, test/spawn-cell.test.ts), which also failed on #10523, need bwrap, socat and unix sockets, which this sandbox doesn't provide. Main reports them green since #10551. The forge runs them on this push.

Checks run: bun scripts/gate.ts and bun x tsc --noEmit pass, and so does bun test on the four files above (58 tests).

🤖 Generated with Claude Code

Approved

Test-only change, no security surface touched. The anchored assertion (alice/x") removes the random-id false match and keeps the positive escape check. The verdict helper wait removes the same-millisecond race against newsSince. The extra sandbox assertions are sound. Clean, no new dependency, no README change. Approved.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs