[birme]

Summary

Adds a -w, --wait option to osc admin sync-fork <serviceId> that waits for
the full two-stage fork-sync-and-rebuild flow to complete, instead of
returning right after the single status check that's currently printed.

sync-fork is not a single async job the way remake-order is — it's two
stages that don't share an id:

1. sync-fork job (maker-manager, GET /sync-fork/by-repo?repositoryUrl=):
   pending → running → succeeded|failed. This does the git-level fork sync.
   On success, maker-manager's service-builder auto-triggers a remake order —
   there's no explicit "start remake" call from the CLI.
2. remake order (maker, GET /maker/:orderId): NEW/PENDING/RUNNING → DONE|FAILED|ERROR. This is the actual image rebuild, and is what
   waitForOrder() (already used by remake-order --wait) polls.

Because the sync-fork job response has no orderId field linking it to the
order that gets created after it succeeds, the correlation between the two
stages is done by looking up maker orders for the same repositoryUrl via
GET /maker?repositoryUrl=... (maker-manager filters server-side by
order.spec.githubRepository). This mirrors the existing pattern in
osaas-app's src/server/actions/builders/serviceUpdate.ts
(getServiceUpdateStatus / findActiveOrder).

Changes

• packages/core/src/platform/maker.ts: new getOrderIdByRepositoryUrl(),
  following the same style/error-handling as the existing
  getOrderIdByName(). Prefers an order in an active phase
  (NEW/PENDING/RUNNING), falls back to the first returned order,
  returns undefined if none found.
• packages/cli/src/admin/syncfork.ts: new waitForSyncFork() that polls
  getSyncForkStatus() every 5s until the job leaves pending/running.
• packages/cli/src/admin/cmd.ts: sync-fork command gains -w, --wait.
  When set: waits for the sync-fork job to finish (reusing the existing
  failed-status output for the terminal-failure case), then polls (bounded
  to ~24 attempts × 5s ≈ 2 minutes) for the remake order to appear via
  getOrderIdByRepositoryUrl, then waits for that order via the existing
  waitForOrder(). Behavior without --wait is unchanged.

Lessons

SyncForkJobStatusResponse/SyncForkByRepoResponse in maker-manager have no
orderId field linking a sync-fork job to the remake order that
service-builder auto-triggers after a successful sync — so --wait has to
poll GET /maker?repositoryUrl=... with a bounded timeout rather than assume
a fixed hand-off point (a fresh order may take a moment to appear after the
sync-fork job reports success). This mirrors an existing pattern already
proven in osaas-app/src/server/actions/builders/serviceUpdate.ts. Also
worth noting: SyncForkByRepoResponse's TypeBox description string says
the terminal success status is "completed", but the actual runtime value
(confirmed in osaas-maker-manager/src/prepare_service/provider.ts:776-782)
is "succeeded" — the description is stale. This PR doesn't hardcode either
string (it only special-cases "pending"/"running" as non-terminal and
"failed" as an error), so it isn't affected, but a future consumer of that
field should not trust the schema description.

Risks Not Tested

No live integration test was run against maker-manager — that would need a
real PAT and a fork that's genuinely behind on its upstream. The bounded
remake-order polling path (sync-fork succeeds but no remake order ever
appears within ~2 minutes) was reasoned through against the maker-manager
source but not exercised against a live environment.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

[birme]

Research Review

Tier: 2 (Standard) — CLI feature touching two integration points (maker-manager sync-fork API, maker order API), no auth/secret/K8s manifest changes.

Verified

• GET /sync-fork/by-repo?repositoryUrl= — confirmed live route in osaas-maker-manager/src/api_sync_fork.ts:237 (SyncForkByRepoQuery/SyncForkByRepoResponse, model.ts:410-447). The PR's SyncForkStatus client-side interface (jobId, repositoryUrl, status, error.{type,message,conflictFiles}) matches SyncForkByRepoResponse field-for-field, including the nested error.type enum values (MERGE_CONFLICT/AUTH_FAILURE/... — model.ts:401-408).
• No orderId field on the sync-fork response — confirmed. Neither SyncForkJobStatusResponse (model.ts:204-224) nor SyncForkByRepoResponse (model.ts:417-447) has any order-linking field. The PR's claim that sync-fork and remake-order are two uncorrelated stages is accurate, not assumed.
• GET /maker?repositoryUrl= — confirmed live in osaas-maker-manager/src/api_maker.ts:176-181, filters server-side on order.spec.githubRepository === request.query.repositoryUrl exactly as the PR body describes. Query param confirmed against MakerQuery schema (model.ts:45-56, repositoryUrl: Type.Optional(Type.String(...))).
• getOrderIdByRepositoryUrl() response shape — reuses the same OutputOrder/Order types (order.orderId, order.status.phase) already used by the pre-existing, presumably-battle-tested getOrderIdByName() in the same file. No new/guessed field names introduced.
• ACTIVE_ORDER_PHASES = new Set(['NEW', 'PENDING', 'RUNNING']) prior art claim — verified byte-for-byte. osaas-app/src/server/actions/builders/serviceUpdate.ts:25 has the identical constant name and value, and findActiveOrder (line 42) implements the identical "prefer active phase order, else fall back to first" logic that getOrderIdByRepositoryUrl reimplements. This is genuine cross-repo pattern reuse, not a coincidence.
• Status-string discrepancy called out in the PR's ## Lessons — independently verified. SyncForkByRepoResponse.status's TypeBox description says "pending, running, completed, failed" (model.ts:420-422), but the actual runtime value set is 'pending' | 'running' | 'succeeded' | 'failed' (prepare_service/provider.ts:776-782, confirmed the exact line range cited in the PR). The PR correctly avoids hardcoding either "completed" or "succeeded" — waitForSyncFork only special-cases pending/running as non-terminal, treating anything else (including the real succeeded value) as terminal. This is evidence of the author actually reading the provider implementation rather than trusting the schema description, which is the opposite of the guessing failure mode this review exists to catch.
• Non---wait code path unchanged — confirmed via diff: the existing sync-fork action body (repo lookup, triggerSyncFork, single status print) is untouched; all new logic is additively wrapped in if (options.wait) { ... }. No regression risk to existing callers/scripts using sync-fork without -w.
• remake-order --wait sibling pattern comparison — cmd.ts:235-270 shows remake-order --wait also calls the same shared waitForOrder() with no additional error handling beyond the outer catch, and also has no bound on waitForOrder's internal infinite poll loop. The new sync-fork --wait path is consistent with this existing style, not introducing a new gap.
• CI: ts (type check) passed. No check-ai-coauthor workflow exists in client-ts (.github/workflows/ only has dispatch.yml, main.yml, pull-request.yml) — the CLAUDE.md-documented coauthor gate is not present in this repo, so its absence here is not a PR defect.

Needs Verification

• packages/core/src/platform/maker.ts — getOrderIdByRepositoryUrl's undefined-status silent pass-through: if waitForSyncFork returns undefined (i.e. getSyncForkStatus 404'd — no sync-fork job found for the repo), cmd.ts:316's guard finalSyncStatus?.status === 'failed' evaluates to false via optional chaining, and the code silently falls through to polling for a remake order with no sync-fork status ever having been confirmed. There's no user-visible message distinguishing "sync-fork job vanished / 404'd" from "sync-fork succeeded." In practice this 404 path should be unreachable immediately after triggerSyncFork succeeds, but it's an unhandled edge case worth a one-line log statement. Low severity — does not block merge, but flagging since the PR's own ## Risks Not Tested section doesn't mention it.
• Unbounded remake-order wait: waitForOrder() itself has no timeout (confirmed pre-existing, maker.ts:82-116) — once the bounded 24×5s orderId-discovery loop succeeds, the final wait can hang indefinitely if the maker service never resolves the order's phase. This matches the existing remake-order --wait behavior exactly, so it's not a regression, but worth noting it's inherited risk rather than newly introduced safety.

Adjacent Stubs Detected

None found. Swept all three touched files (cmd.ts, syncfork.ts, maker.ts) for TODO/FIXME/XXX/HACK, undefined : undefined, empty catch blocks, // removed/// unused/// legacy, and console-log-only handlers — no matches.

Orphaned Files Detected

Not applicable — no new files added; all changes are to existing, already-imported files (cmd.ts is the CLI entry point for the admin command group, syncfork.ts and maker.ts exports are consumed directly by cmd.ts in the same diff).

Selector Reachability

Not applicable — no Playwright/e2e test files touched.

Escape Hatch Coverage

Not applicable — this PR is a CLI polling feature, not an MCP tool or request handler; it introduces no new terminal-rejection branches (isError, throw in a request handler, 4xx response, deny-list entry) reachable by end users. The finalSyncStatus.status === 'failed' branch is a status report to the CLI user based on upstream job state, not a guard blocking a legitimate action.

Impact Analysis

• Blast radius: osc admin sync-fork <serviceId> --wait — an opt-in flag on an admin-only CLI command. No impact on any running service, API contract, or other consumer; purely additive CLI behavior gated behind the new flag.
• Downstream consumers: None — getOrderIdByRepositoryUrl and waitForSyncFork are new exports with a single call site each (cmd.ts), both introduced in this same PR.
• Coordinated changes needed: None. Both backend endpoints (GET /sync-fork/by-repo, GET /maker?repositoryUrl=) already exist and are already deployed in osaas-maker-manager — no backend PR or deploy dependency.
• Breaking change risk: Low. The non---wait path is byte-for-byte unchanged; the new path is additive and opt-in.

Risks Not Tested

• No live run against a real maker-manager instance was performed (by the PR author or this review) — the PR's own ## Risks Not Tested section states this honestly and specifically calls out the untested bounded-timeout path (sync-fork succeeds, remake order never appears within ~2 minutes).
• This review did not execute the CLI locally or write/run a test for getOrderIdByRepositoryUrl — verification here is source-level cross-referencing (diff vs. actual backend route/schema/runtime code), not runtime behavior.
• The interaction between waitForSyncFork's "anything not pending/running/failed is terminal-success" logic and any future third status value (beyond the confirmed pending/running/succeeded/failed) is unverified — if maker-manager ever adds a fifth status (e.g. cancelled), this PR's logic would treat it as success silently. Not a defect today, but worth a note for future maintainers.

Research Suggestions

• Consider adding a one-line Log().debug(...) when finalSyncStatus is undefined (sync-fork status 404'd) so the "no status found, proceeding to poll for remake order anyway" path isn't silent — this is the one gap in an otherwise well-researched PR.
• No action needed on the stale SyncForkByRepoResponse.status schema description ("completed" vs actual "succeeded") from this PR — but consider filing a follow-up doc-fix issue in osaas-maker-manager since the PR's own ## Lessons section flags it as a trap for the next consumer of that field.

Verdict

Approve. This PR is a strong example of researched-not-guessed work: every backend field name, endpoint path, and status value was cross-checked against the actual osaas-maker-manager source (not just the TypeBox schema description, which the author caught being stale), the claimed osaas-app prior-art pattern (ACTIVE_ORDER_PHASES/findActiveOrder) is a verbatim match, the non---wait code path is provably unchanged, and both ## Lessons and ## Risks Not Tested are present, specific, and honest rather than boilerplate. One minor edge case (silent fallthrough on an undefined sync-fork status) is worth a follow-up but does not block merge.

---

Reviewed by PR Research Validator. This review checks whether code is based on verified sources rather than assumptions.