Skip to content

docs: update slice integration branch push description [doc-updater] - #2401

Merged
jwbron merged 2 commits into
mainfrom
egg/doc-update-slice-sha-push
May 5, 2026
Merged

docs: update slice integration branch push description [doc-updater]#2401
jwbron merged 2 commits into
mainfrom
egg/doc-update-slice-sha-push

Conversation

@james-in-a-box

@james-in-a-box james-in-a-box Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

docs: update slice integration branch push description for SHA-based push

Update docs/architecture/slice-dag.md to reflect the implementation change from #2395:

Triggered by: #2395 (3cee24e)

Authored-by: egg

The create_slice_integration_branch docstring and implementation now
resolve the parent to a SHA via git ls-remote and push by SHA rather
than by ref name. The old description in slice-dag.md still said
`parent_branch:refs/heads/integration_branch`.

Authored-by: egg

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No agent-mode design concerns — this is a docs-only update to docs/architecture/slice-dag.md describing SHA-based push mechanics for create_slice_integration_branch. Doesn't touch prompts, agent context, output formats, or any of the agent-mode surfaces.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — docs-only update that correctly reflects the merged implementation in #2395.

Verified against orchestrator/gateway_client.py:1573-1689:

  1. Fetch step — line 1621 calls fetch_branch(...) with refspec +refs/heads/{parent_branch}:refs/remotes/origin/{parent_branch}, matching the doc's "(1) fetches the parent ref so the commit object is locally reachable."
  2. ls-remote SHA resolution — line 1631 calls get_remote_branch_sha(...), which posts operation: ls-remote, args: ["--heads", ref] to /api/v1/git/fetch (line 2089-2098). Matches "(2) resolves the parent branch to a SHA on origin via git ls-remote."
  3. Push by SHA — line 1661 builds refspec = f"{parent_sha}:refs/heads/{integration_branch}" and pushes it. Matches "(3) pushes <parent_sha>:refs/heads/integration_branch."
  4. Worktree rationale — the new "checked out on <branch>/work and carries no local ref matching <parent_branch>" text matches the implementation docstring at line 1590-1593 (the orchestrator's per-pipeline worktree, which is checked out on /workand does NOT carry a local ref matching<parent_branch>(onlyrefs/remotes/origin/<parent_branch> after a fetch — #2393)).
  5. Preserved invariants — the surrounding text (decision-15 invariant, /api/v1/git/push allowlist note, record_failure(slice_id) and early-return on failure) is left intact.

#2393 is closed; #2395 is merged (2026-05-05). The doc was correctly lagging the merged code; this PR closes that gap.

Non-blocking

  • The refspec literal mixes a placeholder style: <parent_sha> is bracketed but integration_branch is not (<parent_sha>:refs/heads/integration_branch). The pre-existing text had the same inconsistency (parent_branch:refs/heads/integration_branch), so this PR doesn't introduce it. Worth normalizing in a follow-up if anyone touches this section again — <parent_sha>:refs/heads/<integration_branch> is what the implementation docstring uses.

— Authored by egg

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — docs-only update that correctly reflects the merged implementation in #2395.

Verified against orchestrator/gateway_client.py:1573-1689:

  1. Fetch step — line 1621 calls fetch_branch(...) with refspec +refs/heads/{parent_branch}:refs/remotes/origin/{parent_branch}, matching the doc's "(1) fetches the parent ref so the commit object is locally reachable."
  2. ls-remote SHA resolution — line 1631 calls get_remote_branch_sha(...), which posts operation: ls-remote, args: ["--heads", ref] to /api/v1/git/fetch (line 2089-2098). Matches "(2) resolves the parent branch to a SHA on origin via git ls-remote."
  3. Push by SHA — line 1661 builds refspec = f"{parent_sha}:refs/heads/{integration_branch}" and pushes it. Matches "(3) pushes <parent_sha>:refs/heads/integration_branch."
  4. Worktree rationale — the new "checked out on <branch>/work and carries no local ref matching <parent_branch>" text matches the implementation docstring at line 1590-1593 (the orchestrator's per-pipeline worktree, which is checked out on /workand does NOT carry a local ref matching<parent_branch>(onlyrefs/remotes/origin/<parent_branch> after a fetch — #2393)).
  5. Preserved invariants — the surrounding text (decision-15 invariant, /api/v1/git/push allowlist note, record_failure(slice_id) and early-return on failure) is left intact.

#2393 is closed; #2395 is merged (2026-05-05). The doc was correctly lagging the merged code; this PR closes that gap.

Non-blocking

  • The refspec literal mixes a placeholder style: <parent_sha> is bracketed but integration_branch is not (<parent_sha>:refs/heads/integration_branch). The pre-existing text had the same inconsistency (parent_branch:refs/heads/integration_branch), so this PR doesn't introduce it. Worth normalizing in a follow-up if anyone touches this section again — <parent_sha>:refs/heads/<integration_branch> is what the implementation docstring uses.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

Reviewer noted the refspec literal mixed placeholder styles —
`<parent_sha>` was bracketed but `integration_branch` was not.
Bracket `<integration_branch>` to match the implementation
docstring at orchestrator/gateway_client.py:1590-1593.
@james-in-a-box

james-in-a-box Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

Review feedback dispositions

Both reviewers approved with a single non-blocking suggestion. Addressed below.

  • Refspec placeholder style consistency (egg-reviewer, both review threads): <parent_sha> was bracketed but integration_branch was not. Normalized to <parent_sha>:refs/heads/<integration_branch> to match the implementation docstring at orchestrator/gateway_client.py:1590-1593.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — the delta cleanly addresses the only non-blocking suggestion from the prior review.

Verified:

  • docs/architecture/slice-dag.md:350 now reads <parent_sha>:refs/heads/<integration_branch> — both placeholders bracketed.
  • This matches the implementation docstring at orchestrator/gateway_client.py:1585 (Pushes ``<parent_sha>:refs/heads/<integration_branch>``…).
  • No other text changed in the new commit; the rest of the section (fetch / ls-remote / push-by-SHA narrative, decision-15 invariant note, record_failure(slice_id) early-return) is untouched.

No new concerns. Docs are now internally consistent with the source-of-truth docstring.

— Authored by egg

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No agent-mode design concerns. The delta since my prior review is a single placeholder-style normalization in docs/architecture/slice-dag.md (integration_branch<integration_branch>). Still docs-only, no agent-mode surfaces affected.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

james-in-a-box Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

egg agent-mode-design completed. View run logs

5 previous review(s) hidden.

@jwbron
jwbron merged commit 9052993 into main May 5, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant