Files
web-terminal/docs/plans/w5-android-parity.md

21 KiB
Raw Permalink Blame History

Android Projects / Diff / Worktree screens (client parity)

Feature id: w5-android-parity · Branch: develop · Server changes: ZERO (every route below already ships from Wave 1-4).

Reality check (read before planning — the task's premise is stale)

The task brief says the Android UI modules are "SDK-gated / commented out in settings.gradle.kts". That is no longer true on develop. Verified on disk:

  • android/settings.gradle.kts:50-53 includes :app, :terminal-view, :host-registry, :client-tls-android (uncommented). The // TODO(android-sdk) gating the README (android/README.md:20-27) still describes is stale doc drift.
  • The SDK is installed and proven: android/local.propertiessdk.dir=/usr/local/share/android-commandlinetools; platforms/{android-35,android-36} present. PROGRESS_ANDROID.md:358-382 records a full green gate (./gradlew test :app:assembleDebug koverVerify → ~484 tests, APK builds). The whole Android client (A1A36) is committed (e254918).
  • :api-client is not empty (only the .gitkeep placeholders remain beside real code): models/Projects.kt, routes/{ApiClient,Endpoints,ApiRoute}.kt etc. exist.

So the actual parity gap is not "bring modules online" — it is: the shipped Android client was built to iOS P0+P1, which deliberately excluded the git-write surface. ANDROID_CLIENT_PLAN.md:79 — "worktree-create — server has routes the iOS client does not consume; out of parity scope"; :233 lists POST /projects/worktree among routes "explicitly NOT consumed"; :218 scopes diff to staged=1|0 only (no ?base=); :221-227 lists only 3 guarded routes (kill / hook.decision / put-prefs). Everything Wave 1-4 added on the web/server is missing on Android:

Server route (exists) src/server.ts Android today Gap
GET /projects/diff?…&base=<rev> 811-837 :app HttpDiffFetcher sends staged only (DiffViewModel.kt:313-322) base-diff
GET /projects/log?path=&n= 843-861 recent commits
GET /projects/pr?path= 871-887 PR + CI chip
POST /projects/worktree 908-934 worktrees shown read-only (ProjectDetailScreen.kt:116-159) create
DELETE /projects/worktree 937-964 remove
POST /projects/worktree/prune 967-986 prune
POST /projects/git/{stage,commit,push} 997-1096 Diff is inert read-only stage/commit/push
GET /projects sync fields ahead/behind/lastCommitMs types.ts:337-339 ProjectInfo (Android) lacks them sync chip

This plan closes exactly that gap. It is the largest Wave 5 item but narrower than "build the client" — the foundation (grouping, prefs round-trip, detail page, diff flatten/render, adaptive nav, mTLS transport, design system) is done and reused verbatim.


Contract (routes / messages / env / types)

No new server routes, no env vars, no :wire-protocol (frozen) changes. All additions are Kotlin client code.

Route home decision (security-driven — do not deviate)

The Origin header (CSWSH 铁律) is stamped in exactly one place: ApiRoute.toHttpRequest() when originPolicy == GUARDED (android/api-client/.../routes/ApiRoute.kt:61-71; ARCHITECTURE §4.3, checklist :467). Therefore every state-changing route MUST be added to :api-client (worktree create/remove/prune, git stage/commit/push) so it flows through that single Origin-stamping point. The RO additions (/projects/pr, /projects/log) also go in :api-client for consistency with the existing RO methods (ApiClient.kt:34-96). The one exception is diff-vs-base: the diff route already lives self-contained in :app (DiffViewModel.kt — A24 put it there because it is read-only and needs no Origin), so extend it in place rather than churning it into :api-client.

New :api-client route builders — routes/Endpoints.kt (mirror the existing 26-93)

// RO (no Origin)
fun projectPr(path: String): ApiRoute      // GET /projects/pr?path=<enc>              READ_ONLY
fun projectLog(path: String, n: Int?): ApiRoute  // GET /projects/log?path=<enc>[&n=<int>]  READ_ONLY
// G (Origin byte-equal) — bodies are application/json, encoded via ModelJson
fun createWorktree(path, branch, base: String?): ApiRoute  // POST   /projects/worktree
fun removeWorktree(path, worktreePath, force: Boolean): ApiRoute  // DELETE /projects/worktree  (BODY on DELETE — see Edge cases)
fun pruneWorktrees(path): ApiRoute         // POST   /projects/worktree/prune
fun gitStage(path, files: List<String>, stage: Boolean): ApiRoute  // POST /projects/git/stage
fun gitCommit(path, message: String): ApiRoute   // POST /projects/git/commit
fun gitPush(path): ApiRoute                // POST   /projects/git/push

Reuse the existing percentEncode (strict RFC 3986 unreserved, Endpoints.kt:102-120) for path/n query values and ModelJson.encodeToString for bodies. n clamps client-side to 1..GIT_LOG_MAX mirror (server re-clamps; send it raw-but-bounded).

New :api-client models (models/, all @Serializable, tolerant decode via ModelJson / LossyDecode, mirror types.ts)

  • PrStatus + PrAvailability (string-union → enum with unknown→ERROR) + PrCheckSummary — types.ts:560-588. Every field except availability optional; availability unknown/missing → ERROR (never throw).
  • CommitLogEntry { hash, at: Long, subject } + GitLogResult { commits, truncated } — types.ts:695-704 (list-lossy: drop malformed commit, keep rest).
  • GitWriteOutcome — a client result union for the guarded git ops carrying the server's safe body: Ok(payload) vs Rejected(status, message). Server always returns { ok, … } on 200 and { ok:false, error:"<safe string>" } / { error } on failure (git-ops.ts:81-119, worktrees.ts:201-341 — never raw stderr). Decode error as an inert string to display.
    • stage 200: { ok, staged: Boolean, count: Int }
    • commit 200: { ok, commit: String } (short sha; may be "")
    • push 200: { ok, branch, remote }
    • worktree create 200: { ok, path, branch }; remove 200: { ok, path }; prune 200: { ok, pruned: List<String> }
  • Extend models/Projects.kt ProjectInfo with ahead: Int?, behind: Int?, lastCommitMs: Long? (types.ts:337-339). Extend the :app DiffResult (DiffViewModel.kt:215) with base: String? = null (types.ts:553).

New ApiClient methods (routes/ApiClient.kt) + status mapping

  • suspend fun projectPr(path): PrStatus — 200→tolerant decode; 400→ProjectPathInvalid; 404→ProjectNotFound; else UnexpectedStatus. (PR degrade — gh missing/unauth/no-PR — is availability inside a 200 body, not an HTTP status.)
  • suspend fun projectLog(path, n): GitLogResult — 200→decode; 400→ProjectPathInvalid; 404→ProjectNotFound; 500→ProjectDetailUnavailable(reuse) or new GitLogUnavailable.
  • Guarded ops return GitWriteOutcome: 200→Ok(...); 403→Rejected reading body.error (disabled and Origin-fail both 403 — see Security); 400/404/409→Rejected(status, body.error); 429→RateLimited; else UnexpectedStatus. New ApiClientError cases as needed: WorktreeDisabled, GitOpsDisabled are not distinguishable from Origin-403 by status alone → prefer surfacing Rejected(status, safeMessage) over inventing typed variants (ApiClientError.kt:11-48 is the sealed set to extend minimally).

:app presenters (plain, JVM-testable — same pattern as DiffViewModel/ProjectDetailViewModel, NOT androidx.lifecycle.ViewModel)

  • WorktreeViewModel(gateway, repoPath): create(branch, base?), remove(worktreePath, force), prune(). Each is a phase machine (Idle/Working/Done/Failed(message)) that re-fetches project detail on success so the worktree list refreshes. Branch name validated client-side (mirror validateBranchName, worktrees.ts:95) before any I/O.
  • Extend ProjectsGateway (ProjectsViewModel.kt:400-413) with the new gateway methods; ApiClientProjectsGateway delegates to the new ApiClient methods. (Keeps the VM JVM-tested against a fake.)
  • Extend DiffViewModel (DiffViewModel.kt:47): add base: String? state + setBase(rev); when base != null the staged toggle is suppressed (server ignores staged when base set — server.ts:831 / public/diff.ts:112-129). Extend diffUrl (:313) to append &base=<enc> and omit staged in base mode. Add stage/commit/push: toggleStage(file, staged), commit(message), push() via the guarded gateway; these are only offered in working/staged mode, never base mode (parity with public/diff.ts:321).
  • PrChipState (fold into ProjectDetailViewModel or a small PrViewModel): fetch /projects/pr lazily on detail load; render one chip.

Files to change

Path Concrete change
android/api-client/.../routes/Endpoints.kt Add 8 builders (pr, log, worktree×3, git×3). RO ones READ_ONLY; write ones GUARDED with ModelJson bodies. Reuse existing percentEncode/HEX.
android/api-client/.../routes/ApiClient.kt Add projectPr, projectLog, createWorktree, removeWorktree, pruneWorktrees, gitStage, gitCommit, gitPush with the status mapping above.
android/api-client/.../routes/ApiClientError.kt Add minimal cases only if needed (GitLogUnavailable); prefer Rejected(status,msg) carried in GitWriteOutcome over per-error variants.
android/api-client/.../models/PrStatus.kt (new) PrStatus/PrAvailability/PrCheckSummary tolerant.
android/api-client/.../models/GitLog.kt (new) CommitLogEntry/GitLogResult list-lossy.
android/api-client/.../models/GitWrite.kt (new) GitWriteOutcome + per-op payloads; serializers registered in models/Serializers.kt if custom.
android/api-client/.../models/Projects.kt Add ahead/behind/lastCommitMs to ProjectInfo.
android/app/.../viewmodels/WorktreeViewModel.kt (new) Create/remove/prune phase machine; branch validation; re-fetch detail on success.
android/app/.../viewmodels/ProjectsViewModel.kt Extend ProjectsGateway + ApiClientProjectsGateway with the new methods; add sync-chip copy to ProjectsCopy.
android/app/.../viewmodels/ProjectDetailViewModel.kt Compose in worktree actions + PR chip fetch + recent-commits fetch (each independent, failure-isolated).
android/app/.../viewmodels/DiffViewModel.kt Add base mode (+setBase), extend diffUrl with &base=, add toggleStage/commit/push via a guarded GitWriteGateway; extend DiffResult with base.
android/app/.../screens/ProjectDetailScreen.kt Add: "New worktree" branch-input sheet (+optional base), per-worktree remove (force-confirm dialog, block isMain), prune button, a PR chip (tappable only if url parses https), a recent commits section. Keep all server strings inert Text (§8).
android/app/.../screens/DiffScreen.kt Add a base-rev input (a third mode beside Working/Staged), per-file Stage/Unstage buttons in working/staged only, a commit-message field + Commit/Push buttons with result banners.
android/app/.../screens/ProjectsScreen.kt Optional: render sync chip (ahead/behind) on project cards from the new ProjectInfo fields.
android/app/.../nav/NavGraph.kt The diff/detail routes already exist (:94-131); no new route needed — worktree/commit UIs are in-place sheets/dialogs, not new destinations. Verify ProjectDetailPane/DiffPane pass the new gateway.
android/app/.../designsystem/* Reuse existing tokens (WebTermColors.statusWorking/statusStuck, WebTermCard, Spacing). PR-check pass/fail/pending → existing status colors. No new tokens.
test files (see TDD) New/extended JVM unit tests under api-client/src/test/... and app/src/test/.../viewmodels/.
android/README.md Fix the stale "SDK-gated / COMMENTED OUT" section (:12-27) to match reality.
docs/PROGRESS_LOG.md Orchestrator appends the W5 entry (not the builder).

TDD steps (ordered — JUnit5 + runTest/StandardTestDispatcher + Turbine + MockK, matching DiffViewModelTest/ApiRouteShapeTest)

Write test → RED → implement → GREEN, bottom-up (pure :api-client first, then :app presenters, Compose deferred to device QA per plan §7).

Phase A — :api-client routes + models (fully green + Kover ≥80% in one pass)

  1. models/PrStatusTest.kt — decode a full availability:"ok" body; unknown availabilityERROR; missing fields tolerated; PrCheckSummary counts; non-object → error/availability=ERROR (mirror HappyPathDecodeTest/TolerantDecodeTest).
  2. models/GitLogTest.kt — decode {commits,truncated}; drop a commit missing hash/at, keep siblings; truncated passthrough.
  3. models/GitWriteTest.kt — decode each 200 payload; decode failure body {ok:false,error:"…"}Rejected(status,message).
  4. routes/GitRouteShapeTest.kt (extend ApiRouteShapeTest) — Origin-iff-guarded: projectPr/projectLog carry no Origin; worktree/git-write carry Origin == endpoint.originHeader byte-equal; assert method+path+query encoding (?path= strict-encoded; &n=); assert DELETE /projects/worktree carries a JSON body.
  5. routes/ApiClientGitTest.kt — with FakeHttpTransport queue canned responses: pr 200/400/404; log mapping; each guarded op 200→Ok, 403→Rejected(body.error), 429→RateLimited, 409→Rejected. Assert the fake received the Origin header on writes and not on reads.
  6. Run ./gradlew :api-client:test :api-client:koverVerify → green.

Phase B — worktree + diff-base presenters (:app, green) 7. viewmodels/WorktreeViewModelTest.kt — invalid branch name → Failed with no I/O; create success → Done + detail re-fetch invoked; remove of isMain blocked; force flag threaded; 403-disabled → Failed(safeMessage); 429 → rate-limited copy. 8. Extend DiffViewModelTest.ktsetBase("main") builds …/projects/diff?path=…&base=main with no staged; base mode hides the staged toggle; diffUrl percent-encodes base; base-mode disables stage/commit/push.

Phase C — git-write from diff + PR/log (:app, green) 9. Extend DiffViewModelTest.kttoggleStage(file,true) posts {path,files:[newPath],stage:true} and refreshes; commit("msg")Ok(sha) banner; empty message rejected client-side or surfaces server 400 message; push()Ok(branch,remote); 409 push-rejected → inert server message. 10. viewmodels/ProjectDetailPrLogTest.kt — PR fetch failure does not fail the detail load (isolated); availability != ok renders degraded copy; recent-commits list decodes + isolates its own error. 11. Run ./gradlew test :app:testDebugUnitTest :app:assembleDebug koverVerify → green.

Device QA (deferred — no emulator/Firebase/host here; append to android/DEVICE_QA_CHECKLIST.md): worktree-create sheet + list refresh, remove force-confirm dialog, Stage/Unstage button layout, commit/push banners, PR chip tap→browser (https-only), base-rev input, sync-chip rendering, adaptive-pane behavior.


Edge cases

  • DELETE with a request body. DELETE /projects/worktree reads req.body (server.ts:943-945), but the current Android transport has only tested body-less DELETE (killSession). Verify OkHttpHttpTransport actually transmits a body on DELETE (some stacks drop it) — add a transport-level test if not covered. This is the single highest-risk integration gotcha.
  • 403 is overloaded. requireAllowedOrigin failure and worktreeEnabled==false/gitOpsEnabled==false both return 403 (server.ts:909-911, 938-939, 998-1000). The client cannot tell them apart by status → must decode body.error and surface it inertly; do not assume 403 means "re-pair / bad Origin".
  • No capability-discovery endpoint. GET /config/ui exposes only allowAutoMode/costBudgetUsd (server.ts:1099-1106) — it does not report worktreeEnabled/gitOpsEnabled/GH_ENABLED. So the client discovers "disabled" only via a 403 at action time; after a disabled-403, hide/disable that control for the session.
  • PR degrade lives in the body, not the status. gh missing/unauth/no-PR all return 200 with availability∈{not-installed,unauthenticated,no-pr,disabled,error} (types.ts:560-566). Render one chip from availability; never treat non-ok as an HTTP error.
  • staged vs base precedence. When base is set the server ignores staged (server.ts:831); mirror by omitting staged and hiding the toggle in base mode (public/diff.ts:112-129).
  • Rate limits (429). stage/commit share gitWriteLimiter; push has a tighter gitPushLimiter (server.ts:1003, 1075). Surface RateLimited copy; do not auto-retry.
  • Empty commit sha. git commit may return {ok:true, commit:""} (git-ops.ts:252) — render "committed" without a sha, don't crash on empty.
  • Base-rev injection. ?base= must be inert and pass the server's isPlausibleRev (server.ts:826); the client sends it percent-encoded and lets the server 400 junk — surface that 400's message.
  • Detail refresh race. A worktree create/remove followed by a detail re-fetch can race a concurrent poll; cancel the in-flight fetch (the job?.cancel() pattern in DiffViewModel.kt:78-82).
  • Force-remove of dirty worktree. Server 409 "uncommitted changes — force required" (worktrees.ts:278) → the remove dialog must offer an explicit Force re-confirm, and must block isMain client-side (worktrees.ts:328).
  • Tolerant decode everywhere. A partial/malformed PR/log/diff body must degrade (drop-one-keep-rest), never throw — reuse LossyDecode/ModelJson discipline (ApiClient.kt:26-28).

Security

  • Origin 铁律 preserved. All six write routes go through ApiRoute.toHttpRequest GUARDED stamping (ApiRoute.kt:61-71) — the single CSWSH defense (ANDROID_CLIENT_PLAN :467). The two new reads (pr/log) MUST NOT carry Origin. A route-shape test asserts both directions (test step 4) so a future reclassification goes red, not silently wrong.
  • Untrusted server strings stay inert. Branch names, worktree paths, commit subjects, PR titles, diff lines, error messages are rendered as plain Compose Text — no linkify/Markdown/AnnotatedString autolink (ANDROID_CLIENT_PLAN :476, plan §8). The PR chip is the one exception: a tappable link only when url parses as https — validate scheme before making it clickable (:476).
  • No credential leakage / no cache. All calls ride the shared mTLS OkHttpClient with .cache(null) (:470) — diff/commit bodies can contain secrets; never persist them.
  • Client-side pre-validation (defense in depth). Branch name (validateBranchName mirror), non-empty commit message, isAbsoluteCwd for any path minted into an action (ProjectsViewModel.kt:139-143 precedent). The server re-validates + realpath-contains regardless (git-ops.ts / worktrees.ts) — the client checks are UX, not the security boundary.
  • Never log worktree paths, commit messages, PR titles, or error bodies.

Effort & dependencies

  • Effort: L (largest Wave 5 item), ~7-9 pd for the buildable+tested layer — below the brief's 8-12 pd because the client scaffold, grouping, prefs round-trip, diff flatten/render, adaptive nav, mTLS transport and design system already exist and are reused verbatim (the app already builds to an APK). The remaining work is ~8 REST routes + ~6 models + 3-4 presenters + ~4 screen edits + ~40-50 tests. Add device-QA time separately (not doable in this env).
  • Dependencies: none external — every server route shipped in Wave 1-4 (server.ts:811-1096), so this is a pure client pass with zero server change. Depends only on the existing frozen :wire-protocol HostEndpoint/HttpTransport and :api-client (both stable). develop must contain the Android tree from e254918 (it does).
  • Can one builder pass get a full green Gradle build? Yes for the logic+compile layer, but phase it into 3 green checkpoints — do not attempt it monolithically. A single pass can realistically finish Phase A (api-client) entirely green with Kover ≥80%. Phases B and C compile (:app:assembleDebug) and pass JVM unit tests, but their Compose surfaces (sheets, dialogs, buttons, chip, banners) are device-QA-deferred by definition (plan §7 — no emulator/Firebase/host in this env), exactly as every prior Android wave deferred rendering. So a realistic single-builder deliverable is: ./gradlew test :app:assembleDebug koverVerify green, with the interactive git-write flows (create a worktree, stage→commit→push from a phone, tap a PR chip) recorded in DEVICE_QA_CHECKLIST.md rather than proven here. Recommended phasing: A (api-client routes/models/tests) → B (worktree + diff-base presenters) → C (git-write-from-diff + PR/log + screen wiring), each ending on a green build so review and rollback stay cheap. The stage→commit→push-from-the-diff-viewer flow (Phase C) is the largest and riskiest single chunk; keep it last and isolated.
  • First action for the builder: run cd android && ./gradlew test :app:assembleDebug to confirm the baseline is green on develop before adding anything (the toolchain is proven but unverified on this exact checkout), and fix the stale README.md no-SDK section as part of the pass.