Hide decision write actions for read-only viewers #293
- 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):

After (archived, Decisions tab):

Before (viewer without push, Document tab):

After (viewer without push, Document tab):

## 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>
- MA
**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.
octocatmentionedlast month on GitHub
octocatsubscribedlast month on GitHub
- MA
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.
lavaman131referenced 57ca51bb763alast month on GitHub
MaggieAppletonhead-ref-force-pushed 65aa80ae8945last month on GitHub
MaggieAppletonhead-ref-force-pushed fbb580ed8a37last month on GitHub
- MA
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 -->
MaggieAppletonhead-ref-force-pushed dcec5828e950last month on GitHub
MaggieAppletonhead-ref-force-pushed b5eeccc0769dlast month on GitHub
MaggieAppletonhead-ref-force-pushed a757b1bfe3falast month on GitHub
- MA
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 -->
MaggieAppletonhead-ref-force-pushed 88e9e0f910b8last month on GitHub
MaggieAppletonreferenced this in Make the decision card the one place to settle, and keep reopening copy truthful #375last month on GitHub
MaggieAppletonauto-rebase-enabledlast month on GitHub
MaggieAppletonmerged 68cb0773ac45last month on GitHub
MaggieAppletonhead-ref-deletedlast month on GitHub