docs: update slice-dag rebase_onto helper reference [doc-updater]

[james-in-a-box]

docs: update slice-dag rebase_onto helper reference for #2542

Update documentation to reflect changes from 685253d:

• slice-dag.md still referenced gateway.git_client.build_rebase_onto_args
  as the argv builder used by GatewayClient.rebase_onto. That import was
  inlined into orchestrator/gateway_client._build_rebase_onto_args in Fix #2535: stop slice-N from inheriting slice-(N-1) consensus, drop gateway import #2542
  to remove the orchestrator image's runtime dependency on the gateway/
  package. Updated the description to reference the new inlined helper and note
  that the gateway server still re-validates args server-side.

The slice consensus isolation fix (also in #2542) is a bug fix; the docs already
described the intended isolation behavior so no further changes were needed.

Triggered by: #2542

Authored-by: egg

[egg-reviewer]

No agent-mode design concerns — this is a docs-only update aligning slice-dag.md with the helper inlining done in #2542.

— Authored by egg

[egg-reviewer]

Review

Doc-only change to docs/architecture/slice-dag.md (+11/-7) that updates the rebase_onto callable description to reflect the inlining done in #2542 (closes #2535).

Verification

I traced every claim in the new text against the current code:

Claim	Verified at
_build_rebase_onto_args lives at orchestrator.gateway_client._build_rebase_onto_args	orchestrator/gateway_client.py:55
Mirrors gateway.git_client.build_rebase_onto_args (same regex, same shape checks, same canonical argv)	gateway/git_client.py:2004
Orchestrator image does not ship gateway/	orchestrator/Dockerfile:15-32 — no COPY gateway/
Canonical argv shape ["--onto", new_base, old_base, branch]	orchestrator/gateway_client.py:101
Client-side rejects flag-shaped, whitespace-bearing, non-git-ref inputs	orchestrator/gateway_client.py:84-99
Gateway server re-validates via validate_git_args("rebase", ...)	gateway/gateway.py:2228 (the /git/execute endpoint runs the allowlist on every submission)
Caller rebase_onto uses the local helper	orchestrator/gateway_client.py:1479
#2535 reference	PR #2542 closes #2535 ✓

The PR body's justification for not documenting the slice-consensus-isolation fix from #2542 is sound — that change was a bug fix; the docs already describe the per-slice isolation behavior the fix restored.

Non-blocking precision note

The doc calls _build_rebase_onto_args "an inlined copy" of the gateway helper. It's not byte-equivalent — the orchestrator version intentionally diverges in two ways documented at orchestrator/gateway_client.py:66-75:

1. Each ref is .strip()-ed in the emitted argv (whitespace tolerated on input, normalised on output)
2. It does NOT call gateway.git_client.validate_git_args (pulling in that import would defeat the point of inlining); the server-side /git endpoint is the authoritative allowlist boundary.

"Inlined copy" is reasonable shorthand and the doc already captures the security-relevant detail (client-side ref-shape check + server-side validate_git_args re-validation), so this is not worth blocking on. Future doc passes could tighten "inlined copy" to "inlined adaptation" or similar if precision matters.

Verdict

No blocking issues. Approving.

— Authored by egg

[james-in-a-box]

Review feedback dispositions

Both reviewers approved the PR. The review bot left one non-blocking precision note; addressing it in-PR per the contract default.

egg-reviewer[bot] (agent-mode-design) — APPROVED

No concerns raised. Nothing to address.

egg-reviewer[bot] (review) — APPROVED with one non-blocking suggestion

1. "Inlined copy" is not byte-equivalent — consider "inlined adaptation"

The doc calls _build_rebase_onto_args "an inlined copy" of the gateway helper. It's not byte-equivalent — the orchestrator version intentionally diverges in two ways: each ref is .strip()-ed in the emitted argv, and it does NOT call gateway.git_client.validate_git_args.

fixed-in-PR (commit 7d9d924). Updated docs/architecture/slice-dag.md:

• Changed "inlined copy" → "inlined adaptation".
• Documented both intentional divergences (mirroring the source docstring at orchestrator/gateway_client.py:65-75): emitted argv is .strip()-ed; helper does NOT call gateway.git_client.validate_git_args.
• Added a parenthetical that the gateway server is the authoritative allowlist boundary; the client-side check is a fast fail, not the security floor.

The reviewer flagged this as non-blocking, but per the feedback contract a "non-blocking" label doesn't count as an explicit defer request and the bias is toward in-PR fixes — small doc tightening costs less than an issue, and accuracy of architecture docs matters.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns — delta since ae1485c is a 7-line precision tightening of the _build_rebase_onto_args description in docs/architecture/slice-dag.md ("inlined copy" → "inlined adaptation", plus the two documented divergences from the gateway version). Doc-only, no impact on agent prompts, tooling, or sandbox boundaries.

— Authored by egg

[egg-reviewer]

Re-review

Delta since ae1485c4 is a single commit (7d9d924) touching only docs/architecture/slice-dag.md (+10/-3). Verified the precision note from my prior review is addressed correctly.

Verification of the new claims

Claim in doc	Verified at
"inlined adaptation" (not byte-equivalent to gateway helper)	Confirmed: orchestrator/gateway_client.py:101 strips each ref in the emitted argv; gateway/git_client.py:2064 does not.
"each ref is .strip()-ed in the emitted argv"	orchestrator/gateway_client.py:101 — ["--onto", new_base.strip(), old_base.strip(), branch.strip()].
"does NOT call gateway.git_client.validate_git_args"	orchestrator/gateway_client.py:55-101 — only the regex shape check runs client-side; gateway helper at gateway/git_client.py:2065 calls validate_git_args, this one doesn't.
"the server is the authoritative allowlist boundary; the client-side check is a fast fail, not the security floor"	Matches the source docstring at orchestrator/gateway_client.py:71-75 ("The gateway server's /git endpoint runs the same allowlist validator on every submission, so the security floor is unchanged").
Reference to orchestrator/gateway_client.py:65-75 for divergence rationale	Docstring at lines 65-75 documents both divergences, matches the doc text.

Precision quibble (non-blocking, not worth a follow-up)

The parenthetical "input whitespace is normalised rather than rejected" is true only for leading/trailing whitespace — interior whitespace is still rejected by the any(ch.isspace() ...) check at orchestrator/gateway_client.py:88. Reading the immediately preceding "each ref is .strip()-ed in the emitted argv" makes the scope clear (.strip() only touches leading/trailing), so this is not actively misleading. Mentioning for completeness only — tightening to "leading/trailing whitespace is normalised" would be marginally more precise but is a wordsmithing pass at best.

No regression risk vs. main

Cross-checked: main's version of this section at docs/architecture/slice-dag.md still describes the pre-#2542 gateway-side build_rebase_onto_args flow, which is now stale. The PR's section update fixes that. The PR branch is behind main on unrelated sections (#2541/#2543/#2546), but those edits are non-overlapping with the rebase_onto callable section, so a merge or squash will compose cleanly.

Verdict

The non-blocking suggestion was addressed thoroughly and accurately. No new issues. Approving.

— Authored by egg

[james-in-a-box]

Review feedback dispositions

Re-review on 7d9d924 approved with one non-blocking precision quibble. Addressing it in-PR per the contract default — small wordsmith, doc-accuracy is cheap.

egg-reviewer[bot] (agent-mode-design, commit 7d9d924) — APPROVED

No concerns raised. Nothing to address.

egg-reviewer[bot] (review, commit 7d9d924) — APPROVED with one non-blocking precision quibble

1. "input whitespace is normalised rather than rejected" overstates the scope

The parenthetical "input whitespace is normalised rather than rejected" is true only for leading/trailing whitespace — interior whitespace is still rejected by the any(ch.isspace() ...) check at orchestrator/gateway_client.py:88. […] Tightening to "leading/trailing whitespace is normalised" would be marginally more precise but is a wordsmithing pass at best.

fixed-in-PR (commit 2dc9f69). Tightened the parenthetical in docs/architecture/slice-dag.md to:

leading/trailing whitespace is normalised rather than rejected; interior whitespace is still rejected by the regex shape check

This now matches the helper's docstring at orchestrator/gateway_client.py:68-70 ("leading/trailing whitespace") and the actual code at orchestrator/gateway_client.py:88 (any(ch.isspace() …) rejects interior whitespace).

The reviewer flagged this as "not worth a follow-up" — per the feedback contract that doesn't count as an explicit defer request, and architecture-doc accuracy outweighs a 4-word edit. Bias toward in-PR fixes.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns — delta since 7d9d9245 is a single commit (2dc9f697) tightening one parenthetical in docs/architecture/slice-dag.md (+3/-2) so the doc distinguishes leading/trailing whitespace (normalised via .strip()) from interior whitespace (rejected by any(ch.isspace())). Pure precision fix, doc-only, no impact on agent design surface.

— Authored by egg

[egg-reviewer]

Re-review (commit 2dc9f697)

Delta since 7d9d924 is a 5-line tightening of docs/architecture/slice-dag.md addressing the non-blocking precision note from my prior review. PR remains docs-only — full PR scope vs merge base (685253df7) is +19/-7 in one file.

Verification of the new claim

The new parenthetical reads: "leading/trailing whitespace is normalised rather than rejected; interior whitespace is still rejected by the regex shape check".

Claim	Verified at
Leading/trailing whitespace is normalised (not rejected)	orchestrator/gateway_client.py:101 — [..., new_base.strip(), old_base.strip(), branch.strip()]. str.strip() only touches leading/trailing.
Interior whitespace is rejected	orchestrator/gateway_client.py:88 (and would also fail the regex at line 94)

The behaviour is correctly described.

Non-blocking precision quibble (please ignore)

Strictly speaking, interior whitespace is gated by the explicit any(ch.isspace() or ch == "\x00" for ch in v) check at orchestrator/gateway_client.py:88, which fires before the regex fullmatch at line 94. The regex (^[A-Za-z0-9._/+-][A-Za-z0-9._/+-]*$) would reject whitespace too — \s is not in the allowlist — so calling it "the regex shape check" isn't wrong, but the actual rejecting gate is the dedicated isspace check. Saying "rejected by the shape check" or "rejected client-side" would be one notch more precise.

This is wordsmithing on top of wordsmithing on a docs-only PR. Not worth a follow-up commit — flagging for the record only.

Verdict

No blocking issues. The previous suggestion was addressed correctly. Approving.

— Authored by egg

[james-in-a-box]

Review feedback dispositions

Re-review on 2dc9f697 approved with one non-blocking precision quibble. Addressing it in-PR per the contract default — small wordsmith, doc-accuracy is cheap.

egg-reviewer[bot] (agent-mode-design, commit 2dc9f697) — APPROVED

No concerns raised. Nothing to address.

egg-reviewer[bot] (review, commit 2dc9f697) — APPROVED with one non-blocking precision quibble

1. "rejected by the regex shape check" understates which client-side gate actually rejects interior whitespace

Strictly speaking, interior whitespace is gated by the explicit any(ch.isspace() or ch == "\x00" for ch in v) check at orchestrator/gateway_client.py:88, which fires before the regex fullmatch at line 94. The regex (^[A-Za-z0-9._/+-][A-Za-z0-9._/+-]*$) would reject whitespace too — \s is not in the allowlist — so calling it "the regex shape check" isn't wrong, but the actual rejecting gate is the dedicated isspace check. Saying "rejected by the shape check" or "rejected client-side" would be one notch more precise.

fixed-in-PR (commit fd26c8f). Tightened the parenthetical in docs/architecture/slice-dag.md to:

leading/trailing whitespace is normalised rather than rejected; interior whitespace is still rejected client-side by the explicit isspace/NUL check that fires before the regex shape check

This now matches the actual code at orchestrator/gateway_client.py:88 (any(ch.isspace() or ch == "\x00" for ch in v)) and makes clear that the regex check is downstream — the dedicated isspace/NUL check is the active rejecting gate.

The reviewer flagged this as "not worth a follow-up commit — flagging for the record only" — per the feedback contract that's a soft observation, not an explicit defer request, and the bias is toward in-PR fixes. Architecture-doc precision is worth the 4-word edit.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns — delta since 2dc9f697 is a single commit (fd26c8f) that tightens one parenthetical in docs/architecture/slice-dag.md (+2/-1), addressing the precision quibble from my prior review. The wording now correctly attributes interior-whitespace rejection to the explicit isspace/NUL check at orchestrator/gateway_client.py:88 rather than the downstream regex. Pure doc precision fix, no impact on agent design surface.

— Authored by egg

[egg-reviewer]

Re-review (commit fd26c8f)

Delta since 2dc9f697 is a single commit (fd26c8f) tightening one parenthetical in docs/architecture/slice-dag.md (+2/-1) per my prior non-blocking precision quibble.

Verification

The new wording reads: "interior whitespace is still rejected client-side by the explicit isspace/NUL check that fires before the regex shape check".

Claim	Verified at
Leading/trailing whitespace normalised (not rejected)	orchestrator/gateway_client.py:101 — emits [..., new_base.strip(), old_base.strip(), branch.strip()].
Interior whitespace rejected by an explicit isspace/NUL check	orchestrator/gateway_client.py:88 — if any(ch.isspace() or ch == "\x00" for ch in v):.
That check fires before the regex shape check	orchestrator/gateway_client.py:88 runs before the _REBASE_REF_RE.fullmatch(v) at line 94.
The regex check is "the regex shape check"	_REBASE_REF_RE is ^[A-Za-z0-9._/+-][A-Za-z0-9._/+-]*$ — a shape-only allowlist, not a semantic git-ref validator.

The wording now correctly attributes the rejection to the active gate (isspace/NUL at line 88) rather than the downstream regex (line 94). This matches the docstring at orchestrator/gateway_client.py:60-75 and the precise code path.

No regression risk vs main

Cross-checked: this PR's change is still confined to a single paragraph in docs/architecture/slice-dag.md. The other unrelated edits to that file on main (#2541/#2543/#2546) do not overlap the rebase_onto callable section, so a merge or squash composes cleanly.

Verdict

The non-blocking precision quibble was addressed accurately. No new issues found in the delta or in the surrounding paragraph. Approving.

— Authored by egg

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

Review feedback dispositions

Re-review on fd26c8f (current HEAD) — both bots approved with no new actionable items raised against the latest commit.

egg-reviewer[bot] (agent-mode-design, commit fd26c8f) — APPROVED

Pure doc precision fix, no impact on agent design surface.

No concerns raised. Nothing to address.

egg-reviewer[bot] (review, commit fd26c8f) — APPROVED (state COMMENTED, verdict approving)

The non-blocking precision quibble was addressed accurately. No new issues found in the delta or in the surrounding paragraph. Approving.

No new feedback items raised. The reviewer confirmed the prior fix landed correctly:

• Wording now correctly attributes interior-whitespace rejection to the explicit isspace/NUL check at orchestrator/gateway_client.py:88 (the active gate) rather than the downstream regex at line 94.
• Cross-checked against main — the change remains confined to one paragraph in docs/architecture/slice-dag.md; unrelated edits to that file on main (Fix #2537: attribute slice PRs to orchestrator, not coder #2541/Fix #2538: slice PRs carry contract.pr narrative on every slice #2543/docs: add --for STATUS to BRC pre-confirm wait example [doc-updater] #2546) do not overlap the rebase_onto callable section, so a merge or squash composes cleanly.

No code changes needed in this round.

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs

14 previous review(s) hidden.