fix(perforce): truncate on max_bytes instead of failing, and size the P-class bounds for real changelists

[ryanlitalien]

Steps 3 and 4 of ButterStack/butter_stack#1904, the connector half. With #11 (already merged) this is the content of v0.3.0.

max_bytes contradicted the protocol

PROTOCOL.md, "Ordering, replay, and size":

Every verb has a max_bytes default; the connector truncates and sets truncated: true rather than streaming unbounded.

run() did not. It short-circuited with perforce: response exceeded max_bytes before a single record was read. The intent was clearly truncation - the maxFiles path immediately below did exactly that - but the byte check fired first.

That is why changelist 118 on project 108 never synced at all rather than syncing partially: 503 files, an ordinary asset reorganisation for a game studio, which is precisely the kind of change this daemon exists to observe.

The bounds were sizing the wrong thing

64 KiB is roughly 500 file records. A cap that small does not protect anything here: p4.describe -s carries no file contents - depot paths, actions, revisions, types - and ButterStack already stores the full file list for every changelist under the limit. The cap never stopped the data existing in the product. It only decided which changelists were allowed to be correct.

What changed

max_bytes shapes the response instead of failing the command. Enforcement moved to where records are assembled. A budget that runs out truncates and sets truncated: true. Each record is charged before it is appended, so the bound holds rather than being discovered one record after it was crossed.

A real memory ceiling replaces it in run(). 64 MiB of raw p4 output per command, enforced by a capped writer rather than by measuring a buffer that has already been allocated - measuring afterwards would already have made the allocation the ceiling exists to prevent. This is a bound on what the daemon is willing to hold, not on what the caller asked for, and no caller can raise it. This is the "megabytes, as a sanity bound" the issue asks for.

Defaults sized for the work: p4.describe 65536 → 4 MiB (~30,000 file records), its max_files schema bound 1000 → 100000, p4.changes 65536 → 1 MiB.

Describe gains depot_file_count - the changelist's real file count, counted before either bound is applied. file_count is what was returned; this is what exists. Without both, a truncated record cannot be told from a complete one by size, and a consumer has no way to know how much it is missing. This is also what lets butter_stack#1904's acceptance criterion 6 become the strict version: a test that fails when a stored file list is shorter than the changelist's own file count.

describeFromRecord is split out of describe() so the bounds - the entire subject of this change - can be exercised without a live p4 server.

Checked and deliberately not changed

internal/tools/teamcity.go:150 shares the 64 << 10 default the issue flagged, but it already truncates correctly: it reads maxBytes+1, compares, slices, and sets truncated. Only the default size matched the pattern, not the hard-fail. Left alone rather than changed for symmetry.

Found in passing

run() decoded straight from the stdout bytes.Buffer, which consumes it - so the stdout.Len() it returned afterwards was always 0. Every successful p4 command has been reporting bytes: 0 to the broker and into the audit log. It now decodes from a reader over the bytes and reports the real length.

Worth noting why this survived: the max_bytes check removed above was the only caller that read the length before the decode drained it. Removing it is what exposed the bug.

Rails side, before rollout

Integrations::Perforce::ConnectorSource::MAX_BYTES still sends 65536. With this change that no longer fails a large changelist, but it would still truncate one - so changelist 118 would re-sync as a flagged partial rather than a complete record. That one-line raise belongs with butter_stack PR #1909 and is called out there; it must land before #1904 step 5 (the re-sync of changes 3, 7, 113 and 118) is attempted.

Tests

go test ./... green, go vet clean, gofmt clean.

Seven new cases: truncation on max_files and on max_bytes (with depot_file_count intact in both), a 503-file changelist coming back whole at the default budget - the case that used to fail outright - a complete list not being marked truncated, the returned records actually fitting the budget they were given, max_bytes: 0 meaning "no preference" rather than "return nothing", and the capped buffer stopping at its limit while still reporting the full write length.

One existing vocab case moved from max_files: 100000 to 100001: it asserts that a bound exists and is enforced at the frame boundary, and the bound moved.