Integrate reviewed contribution workflow improvements

contributing.md

@@ -127,19 +127,81 @@ don't propose.
 Hard stop #1 is the gate. In practice:
 
 1. Propose the contribution through the clarifying-question tool, not as a prose
-   aside. Offer two neutral choices such as **Prepare privately** and **Not
-   now**, and say that nothing goes public without a later approval.
-2. Wait for that yes.
-3. Prepare everything needed for review and direct submission, then stop.
-
-Anything but yes → stop. Preparing is still private: a local branch/commit and a
+   aside. When the partner may still want to improve the candidate, offer three
+   paths: **Prepare privately**, **Refine first**, and **Not now**. Say that
+   preparation stays private and nothing goes public without a later approval.
+2. Wait and classify the response:
+   - **Prepare privately** is approval for preparation only. Prepare everything
+     needed for review and direct submission, then stop.
+   - **Refine first**, or actionable free-text feedback about the candidate,
+     defers the contribution decision; it does not decline it. Do not prepare
+     yet. Apply the feedback within the approved scope, verify the revised
+     change, then offer preparation once for that revised version. If the
+     partner selected **Refine first** without saying what to change, ask for
+     that open-ended feedback in plain chat. Each completed, partner-requested
+     refinement can earn one new offer; never repeat the card when no material
+     work changed.
+   - **Not now** declines preparation. Leave the change local and do not
+     re-offer the same version.
+3. An unanswered, timed-out, disabled, or empty-response card stops the flow
+   without preparing. Silence is neither approval nor refinement, so do not
+   immediately re-ask or treat `{}` / no selection as a yes.
+
+Anything except an explicit **Prepare privately** remains non-approval.
+Refinement feedback changes when the question is asked again, never what the
+agent may publish. Preparing is still private: a local branch/commit and a
 Contribute record, not a fork, push, PR, issue, or comment. The next public step
 happens only after the partner presses **Send PR for review** in Contribute.
 
-An unanswered, timed-out, disabled, or empty-response question card is **not**
-approval. If the review surface expires or returns no answer, leave the
-prepared record private and ask again in plain chat or wait for the Contribute
-Send button. Never treat `{}` / no selection as "yes" for a public action.
+---
+
+## Review the code before every PR
+
+Review the branch after the code is written and **before** you build the review
+commit, so the partner sees the cleaned-up version. Right-size the pass to the
+change: a one-line or docs-only PR needs a careful reread, not a ritual audit;
+a behavioral or structural change earns proportionally deeper review. Reading
+and editing local source needs no approval and publishes nothing.
+
+Use two passes over the branch diff, in order.
+
+**Pass 1 — strip the slop.** Read your own diff as a hostile reviewer of
+machine-written code and delete what a careful human would not have written:
+comments restating what the line already says or breaking the file's existing
+comment style, defensive `try`/`except` and existence checks on paths that are
+already trusted, casts and broad types that only silence a complaint, nesting an
+early return would flatten, and near-duplicates of a helper the codebase already
+has. Behavior stays identical unless you are fixing a clear bug, and the edits
+stay minimal and local.
+
+**Pass 2 — audit the structure the change itself motivates.** Ask whether the
+new behavior sits at the layer that owns it and whether a simpler framing can
+remove branches, flags, helpers, or layers. These are review signals, not
+context-free blockers:
+
+- hand-written source crosses roughly 1000 lines because of the diff (generated
+  files, fixtures, data, and long-form prose do not count);
+- new ad-hoc conditionals or special cases are bolted into a flow that did not
+  care about them before;
+- feature-specific logic leaks into a shared or general-purpose path;
+- an abstraction, wrapper, or layer of indirection is added without buying
+  clarity;
+- optionality, `Any`, or loosely shaped dict payloads paper over an invariant
+  that should be explicit at the boundary;
+- logic lands somewhere other than the layer that already owns the concept, or
+  duplicates a canonical helper;
+- related updates can leave state half-applied, or independent work is
+  serialized for no reason.
+
+Act on findings that are motivated by this change and make the agreed behavior
+clearer or safer, then re-read the diff. Keep unrelated refactors out of the PR.
+When an obvious signal is deliberately left alone, record one sentence in the
+private `plan.prior_work.summary` review evidence — never in the public
+`body_draft` merely to narrate internal process. A larger finding becomes a
+follow-up for the partner rather than silent scope expansion.
+
+If the instance has a richer code-quality or slop-removal skill installed,
+apply it proportionally here too.
 
 ---
 
@@ -153,7 +215,8 @@ Contribute needs to submit it directly after approval:
 ```
 plan: {action: pr|issue|issue_comment|discussion_comment,  # mirrors record.type
        repo, target_url?, title?, body_draft, branch?, repo_path?,
-       base_sha?, head_sha?, diff_sha256?, diff_stat,
+       base_sha?, head_sha?, source_repo_path?, source_sha?,
+       diff_sha256?, diff_stat,
        prior_work?: {searched_at, query, decision, summary?, matches?},
        labels?: [type, area?],
        stack?: {id, name?, position, total, parent_record_id, base_branch},
@@ -209,9 +272,20 @@ plan: {action: pr|issue|issue_comment|discussion_comment,  # mirrors record.type
   longer displayed; you may omit it. Record `base_sha`/`head_sha`/`diff_sha256`
   so the submit button can recompute the exact branch diff before pushing (Hard
   stop #3). Compute the hash from the exact `.diff` bytes you store.
+- For every review originating from an installed app or the platform, record
+  `source_repo_path` (the live source checkout) and `source_sha` (its exact
+  commit when the reviewed diff was captured). The submit path proves
+  `base_sha..head_sha` is present in that source commit and keeps the witness
+  only after the owner sends the PR. If GitHub later merges the reviewed change
+  under a squash/rebase identity, both shell and App Store updates can recognize
+  it as shared history without guessing or dropping later local edits. A linked
+  review already shares the Git objects; for a standalone app review, Contribute
+  imports only the two hash-verified reviewed commits into the installed repo
+  without moving its branch or worktree.
 
 Before you tell the partner it is ready, review the staged record yourself:
-re-read the stored `.diff`, confirm the body draft is exactly what should be
+re-read the stored `.diff`, confirm the proportional quality review above ran
+against the committed branch, confirm the body draft is exactly what should be
 published, confirm no private data appears in the branch, commit message, branch
 name, body, or diff, and confirm the branch is back on `main` when the prep
 steps require it.
@@ -395,6 +469,7 @@ review commit in a linked worktree while the live app stays on `main`:
 SOURCE=/data/apps/<slug>
 WORKTREE=/data/contrib/<record-id>/worktree
 BASE_SHA="$(git -C "$SOURCE" merge-base main upstream)"
+SOURCE_SHA="$(git -C "$SOURCE" rev-parse main)"
 git -C "$SOURCE" -c core.quotePath=false diff --no-ext-diff --no-color \
   --binary --full-index --src-prefix=a/ --dst-prefix=b/ \
   "$BASE_SHA..main" > /tmp/<record-id>.diff
@@ -416,7 +491,8 @@ DIFF_SHA256="$(sha256sum /tmp/<record-id>.diff | awk '{print $1}')"
 ```
 
 Then write the ledger record with `repo_path: "$WORKTREE"`, `branch`,
-`base_sha: "$BASE_SHA"`, `head_sha: "$HEAD_SHA"`, `diff_sha256` from
+`base_sha: "$BASE_SHA"`, `head_sha: "$HEAD_SHA"`,
+`source_repo_path: "$SOURCE"`, `source_sha: "$SOURCE_SHA"`, `diff_sha256` from
 `$DIFF_SHA256`, and `diff_stat` (required). `diff_excerpt` is legacy — omit it.
 
 Two invariants: the
@@ -432,14 +508,20 @@ on `fix/…`, so watcher edits and store updates cannot land on the review branc
 `/data/contrib/<record-id>/worktree` with
 `--separate-git-dir=/data/contrib/<record-id>/git`, `checkout -b fix/…`, copy
 the changed source over (re-read vs the allowlist), and commit with the
-co-author trailer. The separate Git directory is deliberately named `git`, not
-`.git`, so older boot cleaners leave it intact. Use the worktree as `repo_path`.
+co-author trailer. Before cloning, capture the installed app's live source path
+as `source_repo_path` and its exact `main` commit as `source_sha`; the reviewed
+commit identities may differ, and the submit path handles that safely. The
+separate Git directory is deliberately named `git`, not `.git`, so older boot
+cleaners leave it intact. Use the worktree as `repo_path`.
 
 **Platform/shell**: only when `/data/platform` has a real origin — create the
 review branch with `git -C /data/platform worktree add -b fix/…
 /data/contrib/<record-id>/worktree <base-sha>`, apply only the reviewed source
 diff there, and record that worktree path with `repo: "mobius-os/mobius"`.
-`/data/platform` itself remains on `main`. No origin → be honest: platform
+Capture `SOURCE_SHA="$(git -C /data/platform rev-parse HEAD)"` before creating
+the review worktree and store `source_repo_path: "/data/platform"` beside
+`plan.source_sha`; `/data/platform` itself remains on its current live branch.
+No origin → be honest: platform
 contributions need the updated platform bootstrap; app contributions still work.
 
 ## PLATFORM CI
@@ -531,6 +613,7 @@ curl -s -X PUT "$API_BASE_URL/api/storage/apps/<id>/contributions/<record-id>.js
            "body_draft": "<full PR body, word for word>",
            "branch": "fix/<slug>-<short>", "repo_path": "/data/apps/<slug>",
            "base_sha": "<sha>", "head_sha": "<sha>",
+           "source_repo_path": "/data/apps/<slug>", "source_sha": "<sha>",
            "diff_sha256": "<sha256 of the .diff>",
            "diff_stat": "<git diff --stat tail>"}
 }'

mobius.json

@@ -1,8 +1,8 @@
 {
   "id": "contribute",
   "name": "Contribute",
-  "version": "0.5.9",
-  "description": "See what your agent has proposed upstream \u2014 pull requests, issues, and comments across the M\u00f6bius ecosystem, from prepared to merged.",
+  "version": "0.5.10",
+  "description": "See what your agent has proposed upstream — pull requests, issues, and comments across the Möbius ecosystem, from prepared to merged.",
   "author": "mobius-os",
   "license": "MIT",
   "homepage": "https://github.com/mobius-os/app-contribute",
@@ -25,7 +25,8 @@
       "react",
       "react-dom",
       "marked",
-      "dompurify"
+      "dompurify",
+      "@openai/apps-sdk-ui/components/Icon"
     ],
     "esm_deps": []
   },

package.json

@@ -1,6 +1,6 @@
 {
   "name": "app-contribute",
-  "version": "0.5.9",
+  "version": "0.5.10",
   "private": true,
   "scripts": {
     "test": "node --test test/*.test.mjs && python3 -m unittest discover -s test -p 'test_*.py'"

review-followup.md

@@ -82,16 +82,24 @@ you hold the live round. `<base>` below is
 4. **Do the work in the worktree.** Implement in-scope changes. A merge conflict
    requires a history rewrite that the current grant does not authorize:
    escalate it rather than rebasing or force-pushing.
-5. **Run the project's tests** before pushing. If they still fail after two
+5. **Run the proportional quality review** over what you just wrote — the two
+   passes in *Review the code before every PR* in
+   [contributing.md](contributing.md), applied **within this round's scope
+   only**. Your grant authorizes answering the review, not restructuring the
+   codebase: strip the slop and fix local structural problems motivated by your
+   new code. When the structural pass surfaces something genuinely larger, say
+   so in the `/reply` instead of doing it. A finding that needs a rewrite the
+   grant does not cover is an `/escalate`, never a quiet force-push.
+6. **Run the project's tests** before pushing. If they still fail after two
    honest attempts, escalate — don't push red.
-6. **Re-read the FULL diff.** Then write the new `head_sha` and `diff_sha256`
+7. **Re-read the FULL diff.** Then write the new `head_sha` and `diff_sha256`
    onto the ledger record (a CAS storage write, same as preparing) so `/update`
    can bind to exactly what you reviewed.
-7. **Push and reply.** `POST /update` with the new head; then `POST /reply` for
+8. **Push and reply.** `POST /update` with the new head; then `POST /reply` for
    each thread you addressed, using its `in_reply_to` id when it is a review
    thread. Keep replies factual and scoped. Do not mark a draft ready or invent
    a review re-request; those are different GitHub actions.
-8. **Complete.** `POST /complete` with `outcome` and a one-sentence summary.
+9. **Complete.** `POST /complete` with `outcome` and a one-sentence summary.
 
 If a round has nothing to push (a plain question) you may `/reply` then
 `/complete` with `outcome: "replied"`. If you can do neither safely, `/escalate`.

ui/Icons.jsx

@@ -1,18 +1,34 @@
 import React from 'react'
+import {
+  ArrowRotateCw,
+  ArrowUp,
+  Chat,
+  ChevronDown,
+  ChevronRight,
+  SettingsCog,
+  Trash,
+} from '@openai/apps-sdk-ui/components/Icon'
+
+const SDK_ICONS = {
+  send: ArrowUp,
+  feedback: Chat,
+  trash: Trash,
+  settings: SettingsCog,
+  refresh: ArrowRotateCw,
+  chevron: ChevronDown,
+  right: ChevronRight,
+}
 
 const PATHS = {
-  send: <><path d="M12 19V5" /><path d="m6.5 10.5 5.5-5.5 5.5 5.5" /></>,
   merge: <><circle cx="7" cy="5" r="2" /><circle cx="17" cy="19" r="2" /><path d="M7 7v5a7 7 0 0 0 7 7h1" /><path d="M17 17V9a4 4 0 0 0-4-4H9" /></>,
-  feedback: <><path d="M5 17.5 3.5 21l3.8-1.7A9 9 0 1 0 5 17.5Z" /><path d="M8 12h8" /><path d="M8 8.5h5" /></>,
-  trash: <><path d="M4 7h16" /><path d="M9 7V4h6v3" /><path d="m7 7 1 13h8l1-13" /></>,
-  settings: <><circle cx="12" cy="12" r="3" /><path d="M19.4 15a1.7 1.7 0 0 0 .3 1.9l.1.1-2.8 2.8-.1-.1a1.7 1.7 0 0 0-1.9-.3 1.7 1.7 0 0 0-1 1.6v.2h-4V21a1.7 1.7 0 0 0-1-1.6 1.7 1.7 0 0 0-1.9.3l-.1.1L4.2 17l.1-.1a1.7 1.7 0 0 0 .3-1.9A1.7 1.7 0 0 0 3 14H2.8v-4H3a1.7 1.7 0 0 0 1.6-1 1.7 1.7 0 0 0-.3-1.9L4.2 7 7 4.2l.1.1a1.7 1.7 0 0 0 1.9.3A1.7 1.7 0 0 0 10 3V2.8h4V3a1.7 1.7 0 0 0 1 1.6 1.7 1.7 0 0 0 1.9-.3l.1-.1L19.8 7l-.1.1a1.7 1.7 0 0 0-.3 1.9 1.7 1.7 0 0 0 1.6 1h.2v4H21a1.7 1.7 0 0 0-1.6 1Z" /></>,
   github: <path d="M12 2.8a9.2 9.2 0 0 0-2.9 17.9c.5.1.6-.2.6-.5v-1.8c-2.8.6-3.4-1.2-3.4-1.2-.5-1.2-1.1-1.5-1.1-1.5-.9-.6.1-.6.1-.6 1 0 1.6 1.1 1.6 1.1.9 1.6 2.4 1.1 2.9.8.1-.7.4-1.1.7-1.4-2.2-.3-4.6-1.1-4.6-4.9 0-1.1.4-2 1-2.7-.1-.3-.4-1.3.1-2.7 0 0 .8-.3 2.8 1a9.5 9.5 0 0 1 5 0c1.9-1.3 2.8-1 2.8-1 .5 1.4.2 2.4.1 2.7.6.7 1 1.6 1 2.7 0 3.8-2.3 4.6-4.6 4.9.4.3.7 1 .7 1.9v2.7c0 .4.2.6.7.5A9.2 9.2 0 0 0 12 2.8Z" />,
-  refresh: <><path d="M20 11a8 8 0 0 0-14.7-4.3L3 10" /><path d="M3 4v6h6" /><path d="M4 13a8 8 0 0 0 14.7 4.3L21 14" /><path d="M21 20v-6h-6" /></>,
-  chevron: <path d="m8 10 4 4 4-4" />,
-  right: <path d="m9 18 6-6-6-6" />,
 }
 
-export function Icon({ name, size = 18 }) {
+export function Icon({ name, size = 18, ...props }) {
+  const SdkIcon = SDK_ICONS[name]
+  if (SdkIcon) {
+    return <SdkIcon width={size} height={size} className="co-icon" aria-hidden="true" {...props} />
+  }
   return (
     <svg
       className="co-icon"
@@ -25,6 +41,7 @@ export function Icon({ name, size = 18 }) {
       strokeLinecap="round"
       strokeLinejoin="round"
       aria-hidden="true"
+      {...props}
     >
       {PATHS[name] || null}
     </svg>