githubnext/chopinPublic

Let's compose rich plans together

Hide decision write actions for read-only viewers #293

MergedMaggieAppletonmerged 8 commits intomainfromrefs/pull/293/head
  1. MA

    Viewers and archived documents no longer show decision actions that cannot succeed. The open-card link and readable decision state remain available.

    Integration preserves main’s inline-code labels while applying the read-only fallback and action visibility. Reviewed the unchanged style/class boundary before renewing only QuestionView’s exact pin.

    Local checks, types, 118 focused tests, and all eight sidecar/Chat prompt browser tests pass on the integrated candidate. Final hosted CI is pending after #374 lands.

    <details><summary>Original proposal and screenshots</summary>

    ## Why In an archived document, and for viewers without push access, decision cards still showed a disabled "Add an option" row and the chat decision prompt still showed a "Save decision" button. None of it can succeed, so it is misleading.

    ## What changed - `QuestionView` renders the "Add an option" row only when it is given `onAddOption`. Read-only hosts already pass none (and disabled options already have no hover/press state and are not focusable), so this covers the Decisions tab and in-document widgets. - The chat decision prompt hides "Save decision" when `canEdit` is false and says "No option chosen yet" instead of "Choose an option on the card". - The scoped-choice chat prompt hides "Save for this spike" for viewers who cannot edit (it keeps showing "This document is read-only." and the saved state). - New e2e in `e2e/sidecar-option-contracts.e2e.ts`: after archiving, the decision card has no "Add an option"/Save and the chat has no "Save decision". - Footer Discard/Save were already omitted for read-only hosts. No styling changes. - Tests: updated four assertions that expected a disabled add row, added tests for both read-only cases.

    ## Screenshots Before (archived, Decisions tab):

    ![before](https://github.com/user-attachments/assets/08bdc8f5-a8e2-4f85-8536-5393f31a9111)

    After (archived, Decisions tab):

    ![after](https://github.com/user-attachments/assets/90015506-0920-4391-8ed3-7f267e6e6b68)

    Before (viewer without push, Document tab):

    ![before viewer](https://github.com/user-attachments/assets/605d0a4b-d81f-49ad-a12a-03cab07db2e9)

    After (viewer without push, Document tab):

    ![after viewer](https://github.com/user-attachments/assets/810604c2-402b-421b-8aad-71e60c8577b0)

    ## Testing - `bun test packages/question packages/editor apps/web`: all pass. `apps/web/src/focus.test.ts` timed out once under load in my first full run, passes alone here and on a fresh `origin/main` worktree (not a regression). - `bun run types`: pass. `bun run fix`: no changes. - `bun run ci`: fails only on the design-contract review hash (below). Local e2e is suspended; CI runs the browser suite.

    ## Needs a design-contract review - `packages/question/src/react/question-view.tsx`, exception file `scripts/design-contract/exceptions/dynamic-packages.json`: new sourceHash `b6a9df1a3701b152bdbfb0b3d603fb8da7e20828a05c8cff8647c4b7ac072444` (previously `6ffb7d6df2fc865f4eb0803a0e79fa348663f78f57e7040567d5b45146037a2c`). The change only wraps an existing component in a condition; no class or style logic changed.

    🤖 Generated with [Claude Code](https://claude.com/claude-code)

    </details>

  2. MA
    Author

    **Review:** Changes needed

    The behaviour is right and the e2e job is red on stale assertions this PR did not update.

    **Browser check (read-only handle `readonly`, and writer `octocat` on an archived doc, desktop 1440x900, Document tab widgets and Decisions tab, including the expanded "resolved" list):** - No "Add an option", Save or Discard remains for either viewer. The option radios are disabled, which is the existing read-only treatment. The "Show source in chat" icon is an enabled navigation control, not a write. - Recorded answers still show: the "Decided: On disk as MDX · @octocat" line and the Answered-by row under "resolved". - Writer on an active doc still sees "Add an option", Discard and Save on every open card. - Not exercised in the browser: the chat decision prompt and scoped-choice entries. The fake server ran with `AGENT=off`, so no Planner prompts could be produced. Those are covered only by the new unit tests, which look right. The new e2e's `Save decision` count-0 assertion is vacuous when no prompt is in chat.

    **Blocking: e2e fails (job 111706765388).** 1. `e2e/sidecar-option-contracts.e2e.ts:226`, in "a read-only reader cannot change a decision", still has `expect(Add an option).toBeDisabled()`. The element is now absent. Change it to `toHaveCount(0)`. This is the failure CI reports. 2. Two more assertions on the same stale pattern will fail once their project runs; CI stopped at 1 failure, so these are not yet shown: - `e2e/conversation-plan-ui.e2e.ts:24-25`, in `expectOpenCardReadOnly`: `add` `toBeDisabled()` should be `toHaveCount(0)`. - `e2e/conversation-plan-prompts.e2e.ts:507`: the reader's `Save decision` `toBeDisabled()` should be `toHaveCount(0)`. Assert "No option chosen yet" instead if the prompt text matters. Grep e2e for other read-only `Add an option`, `Save decision` and `Save for this spike` expectations before the re-run.

    **CI lint job:** fails only on the design-contract review hash for `packages/question/src/react/question-view.tsx`. This is acceptable per the process. The change is a single `onAddOption &&` condition, and the PR body already lists the hash.

    **Code:** the `question-view.tsx`, `decision-entry.tsx` and `scoped-choice-entry.tsx` changes are minimal and use no new styles. `(canEdit || saved)` correctly keeps the saved state visible. No other issues.

  3. octocatmentionedlast month on GitHub

  4. octocatsubscribedlast month on GitHub

  5. MA
    Author

    Updated the stale e2e assertions to expect absent controls (toHaveCount(0)): sidecar-option-contracts (read-only reader), conversation-plan-ui (expectOpenCardReadOnly), conversation-plan-prompts (reader Save decision), and e2e/source/question-view-contracts.native.ts (Add an option and chat Save decision). No other disabled assertions on these controls remain; the remaining Save for this spike toBeEnabled checks are for writers. I dropped the chat Save decision line from the new archived-doc test because the seeded document has no chat prompt, so it was vacuous. CI run 37293689570 on 532d7b5e is queued; the design-contract hash failure in the validation job remains, as described in the PR body.

  6. lavaman131referenced 57ca51bb763alast month on GitHub

  7. MaggieAppletonhead-ref-force-pushed 65aa80ae8945last month on GitHub

  8. MaggieAppletonhead-ref-force-pushed fbb580ed8a37last month on GitHub

  9. MA
    Author

    Confirmed from failed `ci` run `37618123973`: the first meaningful failure is `Format, lint, and enforce design tokens`, and the only hard failure is stale reviewed dynamic-owner exceptions in `packages/question/src/react/question-view.tsx` (reported at lines 439, 440, 456, and 548). `oxlint` emitted warnings only, and `e2e` plus `container` were green on this head.

    I did not push a speculative fix because this runner cannot install or run the pinned Bun 1.4.2 toolchain: `bun` is absent here, and downloading the public 1.4.2 release binary was blocked, so I could not rerun `bun install --frozen-lockfile`, `bun run fix`, or `bun run ci` locally to verify an exact reviewed-hash renewal.

    Next action for a human: on a machine with Bun 1.4.2 available, check out `fbb580ed8a37a924795b96e29ee0936c8cf19faa`, run `bun install --frozen-lockfile` and `bun run ci`, inspect the `question-view.tsx` data-flow change, and renew the exact entry in `scripts/design-contract/exceptions/dynamic-packages.json` only if that review still looks safe.

    `babysit-head:fbb580ed8a37a924795b96e29ee0936c8cf19faa`

    > [!WARNING] > <details> > <summary>Firewall blocked 1 domain</summary> > > The following domain was blocked by the firewall during workflow execution: > > - `releaseassets.githubusercontent.com` > > To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "releaseassets.githubusercontent.com" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > > </details>

    > Generated by [PR CI fixer](https://github.com/githubnext/chopin/actions/runs/37618994335) · gpt54 · 59.8 AIC · ⌖ 5.54 AIC · ⊞ 16.4K · [◷](https://github.com/search?q=repo%3Agithubnext%2Fchopin+%22gh-aw-workflow-call-id%3A+githubnext%2Fchopin%2Fpr-ci-fix%22&type=issues)

    <!-- gh-aw-agentic-workflow: PR CI fixer, engine: codex, model: gpt-5.4, id: 37618994335, workflow_id: pr-ci-fix, run: https://github.com/githubnext/chopin/actions/runs/37618994335 --> <!-- gh-aw-workflow-call-id: githubnext/chopin/pr-ci-fix -->

  10. MaggieAppletonhead-ref-force-pushed dcec5828e950last month on GitHub

  11. MaggieAppletonhead-ref-force-pushed b5eeccc0769dlast month on GitHub

  12. MaggieAppletonhead-ref-force-pushed a757b1bfe3falast month on GitHub

  13. MA
    Author

    Confirmed from failed `ci` run `37632475485`: the current head still matches `a757b1bfe3fafcba6cb8318256e0b9e1fbcea363`, `e2e` and `container` are green, and the only failing check is `format, lint, types, tests`. Based on the current PR state and the existing validation failure description, this still looks like a reviewed design-contract exception update for `packages/question/src/react/question-view.tsx`, not a behavior regression elsewhere in the branch.

    I did not queue a commit from this runner because the pinned local toolchain is unavailable here: `bun` and `mise` are both missing, so I could not run the required `bun install --frozen-lockfile`, `bun run fix`, or `bun run ci` reproduction and verification steps.

    Next action for a human: on a machine with Bun 1.4.2 available, check out `a757b1bfe3fafcba6cb8318256e0b9e1fbcea363`, rerun `bun install --frozen-lockfile` and `bun run ci`, then renew the exact reviewed exception in `scripts/design-contract/exceptions/dynamic-packages.json` only if the `question-view.tsx` ownership change still looks safe.

    `babysit-head:a757b1bfe3fafcba6cb8318256e0b9e1fbcea363`

    > Generated by [PR CI fixer](https://github.com/githubnext/chopin/actions/runs/37627304799) · gpt54 · 35.4 AIC · ⌖ 4.99 AIC · ⊞ 16.4K · [◷](https://github.com/search?q=repo%3Agithubnext%2Fchopin+%22gh-aw-workflow-call-id%3A+githubnext%2Fchopin%2Fpr-ci-fix%22&type=issues)

    <!-- gh-aw-agentic-workflow: PR CI fixer, engine: codex, model: gpt-5.4, id: 37627304799, workflow_id: pr-ci-fix, run: https://github.com/githubnext/chopin/actions/runs/37627304799 --> <!-- gh-aw-workflow-call-id: githubnext/chopin/pr-ci-fix -->

  14. MaggieAppletonhead-ref-force-pushed 88e9e0f910b8last month on GitHub

  15. MaggieAppletonreferenced this in Make the decision card the one place to settle, and keep reopening copy truthful #375last month on GitHub

  16. MaggieAppletonauto-rebase-enabledlast month on GitHub

  17. MaggieAppletonmerged 68cb0773ac45last month on GitHub

  18. MaggieAppletonclosedlast month on GitHub

  19. MaggieAppletonhead-ref-deletedlast month on GitHub