Skip to content

Phase 4 — Git / workflow hardening (rides 0.19.0)#318

Merged
rennf93 merged 86 commits into
fix/scan-data-integrityfrom
fix/scan-git-workflow
Jul 7, 2026
Merged

Phase 4 — Git / workflow hardening (rides 0.19.0)#318
rennf93 merged 86 commits into
fix/scan-data-integrityfrom
fix/scan-git-workflow

Conversation

@rennf93

@rennf93 rennf93 commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Phase 4 — Git / workflow hardening (rides 0.19.0)

Fourth phase of the 2026-07-06 full-codebase-scan fix program. Eight verified git/workflow-hardening findings + an e2e smoke module, all under the existing ## [0.19.0] CHANGELOG section. Stacks on Phase 3 (fix/scan-data-integrity, PR #317) — base is fix/scan-data-integrity; merge #317 first.

Security

  • H11 (a265fe59, dcac700c, 71484358) — the clone/fetch/ls-remote/push PAT is injected via git -c http.extraheader=Authorization: Basic <base64(x-access-token:token)> instead of URL-embedding (https://TOKEN@host/...). The raw PAT no longer appears in argv, so /proc/<pid>/cmdline can't expose it. Applied across workspace.py (clone + read-clone fetch) and release_executor.py (clone + push + ls-remote); the root-cause helper _inject_token_into_url is deleted. SSH URLs and tokenless public repos are byte-for-byte unchanged.
  • H8 (bc714180, 9445fb7b) — rebase_onto_base gates on a clean tree (git status --porcelain empty else ValidationError(DIRTY_WORKSPACE)), mirroring pull, so a rebase never silently discards uncommitted agent edits. rebase --abort stays valid recovery because the rebase never starts on a dirty tree. Test mocks updated for the new leading status call.
  • H9 (43c52193) — _link_commit_to_task calls flush() not commit(), so it no longer commits the verb runner's savepoint / drags in pending orchestrator state out-of-band.

Fixed

  • M37 (104532df) — merge_pr locks the work_session row FOR UPDATE (with_for_update(of=WorkSessionTable)) before the status != ACTIVE idempotency check, so two concurrent merge_pr calls (retried CEO double-click racing PM auto-complete) serialize — only one writes merged_by/pr_merged_at. The of= scoping is load-bearing: WorkSessionTable.project/.agent are lazy="joined", so without it Postgres rejects FOR UPDATE on the nullable join side with FeatureNotSupportedError (mirrors the Phase 2 task.py:2757 precedent).
  • M38 (45dd94b3) — _pr_is_merged returns None (not False) on httpx.HTTPError; callers treat None as "assume merged" and fall through to the merged-cleanup path, so an indeterminate gh call doesn't re-spawn the PM against an already-merged PR. is_pr_merged_for_task keeps a bool return via True if merged is None else bool(merged).
  • M39 (43be8384) — _cherry_unmerged_entry's marker grep is anchored to ^\[{id[:8]}\] (leading ^ + trailing space), so a commit that merely references the child (e.g. "Reverts [CHILDID8]") no longer matches → a child's unmerged commits are no longer hidden from assembled-PR integrity.
  • L1 (a97ebe3c, 6a65553f) — actor_agent_id is threaded through update_pr_for_task_link_commit_to_task, and _resolve_workspace_agent_id is narrowed to actor → assigned_to → None (the created_by fallback is removed). The post-handoff Main-PM path still works because pr_merge's three choreographer callers thread the real actor_agent_id; pr_target has zero production callers (test-only); is_pr_merged_for_task's None path is a pre-existing fragility the old created_by path shared. Two stale docstrings refreshed to drop the removed created_by fallback.
  • L2 (33bcba3b) — push(force=True) uses --force-with-lease (not bare --force), so a concurrent remote advance fails fast instead of being silently clobbered.

e2e smoke

  • tests/e2e_smoke/test_git_workflow.py (46347633) — 4 cross-layer scenarios: H11 (PAT not in argv, http.extraheader present), M37 (two concurrent merge_pr calls serialize — exactly one merger), M38×2 (_pr_is_merged returns None on httpx.HTTPError; _merge_with_retry falls through without raising MergeConflictError). H8 skipped (a real-worktree clean-tree gate is infeasible under the harness's session-scoped workspace contamination; H8 is unit-covered by test_git_rebase.py). Deviations documented in the module docstring.

Verification

  • uv run ruff format + ruff check — clean on all Phase 4 changed files.
  • uv run mypy roboco/services/{git,work_session,workspace,release_executor}.py — clean.
  • Unit suites green: test_git.py + test_git_rebase.py (65 passed), test_work_session.py (13 passed), test_workspace_read_clone.py + test_workspace_clone_pat_leak_cleanup.py (16 passed).
  • make e2e-smoke — every Phase 4 scenario passes in isolation (4 passed). The full-suite session-scoped workspace contamination across e2e files is a pre-existing harness limitation, documented + out of scope.
  • Whole-branch opus review: Fix then push → the one Critical (C1, a ~50%-flake in the M37 concurrency tests — order-dependent assertions assumed caller A wins the FOR UPDATE race, but which caller wins is non-deterministic; production M37 code was correct) is fixed in 7347e29c by asserting the invariant (both rows COMPLETED, both report the same merged_by, value ∈ {a_merger, b_merger}) in both the unit and e2e tests. Two Task-9 Minors folded into the same commit (H8 e2e skip now documented in the module docstring; internal "Phase 4" label dropped from the docstring header in favor of the public "0.19.0" version anchor). No production code changed in the fix commit. The other 7.5 fixes confirmed clean: H11 (zero dangling _inject_token_into_url refs), H8, H9, M38 (None handled at all 3 callers), M39, L1 (no caller depends on the removed created_by fallback; the one None-actor site is a strict improvement), L2, M37. All 13 commits signed, no co-author, no new # type:ignore/# noqa, no new migration/subsystem, AGPL-3.0 only.

Model note

Implementers ran on sonnet; whole-branch review on opus. kimi-k2.7-code:cloud is not routable in this harness.

🤖 Generated with Claude Code

rennf93 added 14 commits July 6, 2026 23:11
Sibling site to the clone fix: the conventions read-clone refresh ran
'git fetch --tags <https://TOKEN@host> <branch>', exposing the PAT in the
fetch argv on the orchestrator host. Mirrors the clone site's per-call
'-c http.extraheader=Authorization: Basic …' prefix + bare URL. SSH URLs
and tokenless public repos unchanged.
H8 inserted a 'git status --porcelain' dirty-tree gate at the top of
rebase_onto_base (mirroring pull). The 3 rebase control-flow tests mocked
_run_git with a side_effect list matching the OLD call sequence (no
leading status call), so every call shifted by one and the assertions
missed. Prepend a clean-status result to each list so the gate passes
and the fetch/checkout/reset/rebase/diff/abort/push sequence aligns.
Verified: 16 passed (was 3 failed/13 passed post-H8, 16 passed pre-H8).
pr_target and _workspace_for_branch still documented the actor →
assigned_to → created_by fallback chain that L1 removed from
_resolve_workspace_agent_id. Update both to the post-L1 actor →
assigned_to → None resolver (project.workspace_path as the final
fallback) so a future reader doesn't rely on a fallback that no
longer exists.
The opus whole-branch review flagged the M37 concurrency tests as
~50% flaky: both asserted caller A wins the FOR UPDATE race, but
which caller wins the lock is non-deterministic. When B won, the
'assert a_row.merged_by == a_merger' branch flipped false even
though the production code (M37) was correct — exactly one merger
recorded, audit trail intact. Assert the invariant instead: both
rows COMPLETED, both report the same merged_by, value in
{a_merger, b_merger}. Applied to both the unit test and the e2e
twin. Also documents the H8 e2e skip in the module docstring (the
report claimed it was documented there but it wasn't) and drops
the internal 'Phase 4' label from the docstring header in favor of
the public '0.19.0' version anchor.
@github-project-automation github-project-automation Bot moved this to Backlog in RoboCo Kanban Jul 6, 2026
@github-actions github-actions Bot added documentation Docs, README, CHANGELOG, governance files area: services Touches roboco/services/ (business logic, side effects) tests Test suite changes area: gateway Touches roboco/services/gateway/ (Choreographer, verb surface) labels Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Thanks for opening your first pull request on RoboCo!

Quick checklist before review (most of these are enforced by CI, but worth a glance):

  • make quality — ruff format check, ruff check, mypy, pytest (≥80% coverage), and the rest of the gate
  • Panel changes pass pnpm lint and pnpm exec tsc --noEmit (run from panel/)
  • No # noqa / # type: ignore shortcuts; pre-existing violations in touched files are fixed
  • Added an entry under ## [Unreleased] in CHANGELOG.md
  • Signed the CLA (the bot will prompt you on this PR)
  • Signed your commits — master requires verified signatures (SSH signing setup)
  • Updated any affected docs under docs/

See CONTRIBUTING.md for the full workflow and the Code of Conduct for the community standards we follow.

Welcome aboard — a maintainer will review shortly.

rennf93 and others added 21 commits July 7, 2026 06:29
…ep so live cost reflects Anthropic cache spend
[phase9] Auth hardening: bound agent tokens (iat/exp) + JWT jti revocation
[scan-fix] Phase 8: LLM providers / usage / billing
[Phase 7] schemas/conventions/MegaTask/API fixes (15 findings, scan-fix program)
Phase 6 — Panel frontend hardening (rides 0.19.0)
Phase 5 — Background-engine hardening (rides 0.19.0)
@github-actions github-actions Bot added dependencies pyproject.toml / uv.lock or panel package manifests area: panel Touches panel/ (Next.js control panel) area: api Touches roboco/api/ (FastAPI routes, schemas, app) area: db Touches roboco/db/ or roboco/models/ area: alembic Touches alembic/ (database migrations) area: orchestrator Touches roboco/runtime/ (agent spawner, dispatch loops) labels Jul 7, 2026
@rennf93 rennf93 merged commit 009288a into fix/scan-data-integrity Jul 7, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in RoboCo Kanban Jul 7, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: alembic Touches alembic/ (database migrations) area: api Touches roboco/api/ (FastAPI routes, schemas, app) area: db Touches roboco/db/ or roboco/models/ area: gateway Touches roboco/services/gateway/ (Choreographer, verb surface) area: orchestrator Touches roboco/runtime/ (agent spawner, dispatch loops) area: panel Touches panel/ (Next.js control panel) area: services Touches roboco/services/ (business logic, side effects) dependencies pyproject.toml / uv.lock or panel package manifests documentation Docs, README, CHANGELOG, governance files tests Test suite changes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant