Skip to main content
On this page

P1 · GitHub-first migration — progress note (2026-05-21)

Status: SHIPPED (locally). Awaiting your push.

Commits on v4/P1-github-first

Text
4dc7982 v4/P1-review-fix: restore legacy meta back-compat in head_sha fallbackdb183e1 v4/P1-W2-finish: complete adk-cli rename + remaining files026da16 v4/P1-W4: docs + tests rename + new read-shim/bare-number tests5096092 v4/P1-W3: pr-review scripts + indexer rename326c98d v4/P1-W2-partial: pr_queue.py terminology rename + bare-number adde3b1a17 v4/P1-W1: core types + read-shim7a1c4de v4/P0: baseline carryforward (pre-v4 WIP)

Range diff for review: git diff e1e7b2f..4dc7982 --stat.

Test pass count

python3 -m pytest skills/adk-cli/scripts/tests/ skills/adk-pr-review/scripts/tests/ → 321 passed.

Includes two new exit-criterion tests:

  • test_legacy_field_normalisation.py — 1 test, read-shim idempotency.
  • test_pr_queue_add_bare_number.py — 4 tests, github + bitbucket + #-prefix + missing-defaults.

Exit-criteria check (plan §8 P1)

Criterion Evidence
adk pr-queue add 1234 works against defaults.repo pr_queue.py:_resolve_bare_pr_number + 4 passing tests. Resolver reads ~/.agents-devkit/config/core.yaml defaults.platform (default github) + defaults.repo freshly each call (no import-time path binding, so tests with monkeypatched HOME work).
All tests green 321 passed.
No legacy term in user-visible strings Tree-wide grep clean modulo: read-shim renames table (queue_io.py docstring), STATUS_DECLINED aliased import, host-API verbatim words (DECLINED/SUPERSEDED/CLOSED in classify_pr_state), n_pr_links_in_message (Slack metadata, different field), pr_links (list-of-URLs variable).
Aliases live for one release STATUS_DECLINED = STATUS_CLOSED in queue_io.py:62. Documented in plan §8 P1.
In-place migration of user’s queue Implemented as idempotent read-shim in queue_io.read_queue() (renames + writes back on next save). User’s live Bitbucket-shaped queue (10+ rows) survives without P7.

Code review verdict

PASS — P1 ships (one should-have finding, addressed in commit 4dc7982).

Reviewer notes (full text in conversation log):

  • should-have — duplicated or from bulk-sed pass; restored legacy meta back-compat. ✓ fixed.
  • may-have — repo.py:345 still emits head_oid in indexer summary dict (P3/P5 scope; not user-visible queue/state field).
  • nit — STATUS_NEEDS_FIX alias predates P1; deprecation clock now ticks on both aliases.

Deferred to later phases

Item Phase
repo.py:345/359/378 indexer summary dict still uses head_oid (internal JSON, not queue) P3 (repo layout overhaul)
STATUS_DECLINED + STATUS_NEEDS_FIX alias removal Post-v4 (one-release clock)
prepare_task.py rename (was run_review.py) P5 (plan §8 P5)
defaults.repo + defaults.platform documentation in SETUP.md / core.yaml template P6 (CLI completeness) — drafted but not yet integrated

Open questions for the user

  1. The 26 worker-iteration failures we saw mid-integration were caused by my own bulk-sed missing some sites — they’re now all green. No blocker, but if you want the granular history (commit by commit), the topic branch has it.
  2. The plan’s §11 P1 column for /prj-adk-pr-auto says “n/a” but the skill consumes verbose-mode CLI verbs (P6’s deliverable). Doesn’t affect P1, just calling it out.

Exact commands to inspect

Shell
git log --oneline 7a1c4de..4dc7982    # 6 commits on top of baselinegit diff e1e7b2f..4dc7982 --stat      # full P1 diff vs pre-v4 HEADgit diff e1e7b2f..4dc7982 -- skills/adk-cli/scripts/queue_io.py | lesspython3 -m pytest skills/adk-cli/scripts/tests/ skills/adk-pr-review/scripts/tests/ -q