docs(skill): prefer preview environments for development

[ralt]

The skill now makes preview environments the default place to develop, test, and review changes on Upsun, including debugging, database migrations, and performance experiments. Production container logs under /var/log are not cloned, so scoped reads through SSH or upsun logs remain available without first reproducing the issue in a preview.

Adds a focused reference covering external-service isolation, parent-to-child sync, repeatable migration tests, resource comparisons, and cleanup or retention for review. Other production SSH access remains a last resort.

Validation: skill validator and git diff --check pass. Hosted LLM evaluations were not run locally.

[Copilot]

🟡 Changes recommended

Unresolved critical workflow issues and a moderate log-command issue must be corrected before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR makes Upsun debugging, migration, and performance work preview-first while retaining scoped production log access.

Changes:

• Adds preview isolation, synchronization, migration testing, performance comparisons, and cleanup guidance.
• Updates tunnel, logging, branching, migration, resource, and confirmation workflows.
• Adds detailed preview-environment procedures.

File summaries

File	Final findings
plugins/upsun/skills/upsun/SKILL.md	Critical (1 vote): Preview SSH/write steps exceed declared capabilities. Moderate (1 vote): Log commands omit the application target and required --tail form.
plugins/upsun/skills/upsun/references/preview-environments.md	Critical (2 votes): Sync examples must use explicit --data and --code options.

Review details

Suppressed comments (1)

plugins/upsun/skills/upsun/SKILL.md:182

• Both new commands omit the app target used by the skill's documented log invocation at line 118, and the production command also omits --tail. As written, the workflow does not reliably request the application container log needed for the production/preview comparison; keep the application target (and the existing tail form) in both commands.

- For production log evidence, read the relevant logs with `upsun logs -p <PROJECT_ID> -e <PRODUCTION_ID>` or scoped SSH reads of `/var/log`. These logs are not cloned into previews; the [container-log exception](#prefer-preview-environments-for-experiments) allows reading them directly.
- Reproduce the issue and test fixes in a preview, inspecting that preview's own logs with `upsun logs --tail -p <PROJECT_ID> -e <PREVIEW_ID>`.

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[upsun-dispatch]

Warning

Changes suggested — 🟡 2 warnings · 🔵 3 minor points

🔍 Full review · 2 files reviewed

Verification

• The three new intra-file links resolve to the new heading ## Prefer preview environments for experiments (anchor prefer-preview-environments-for-experiments).
• The two links into the new reference file target headings that exist there: #test-a-database-migration and #compare-performance.
• The sync guidance direction (parent into child, preview as -e target) matches the table's per-selection effects and the new upsun sync entry in the write-confirmation list.
• The reference file's deactivation statement agrees with SKILL.md's safety rule that deactivate removes services and data but keeps the branch.
• The new production-log guidance stays within the frontmatter allowed-tools glob Bash(upsun logs*); only SSH falls outside it.

Docs-only change with no tests added or changed; the repository's only automated check is .github/workflows/run-evals.yml, which installs the skill and runs the DeepEval suite in evals/ — that suite contains a single login test (evals/test_login.py) and exercises none of the preview, log, tunnel, or sync guidance this PR rewrites, so nothing verifies the new behaviour.

Review details

• Commit: ed8c805
• Model: claude-opus-5

---

Review 1 of 10 for this pull request · View the full run

[upsun-dispatch]

Note

Reviewed — No new issues found · 5 still open

🔁 Incremental · 2 files reviewed

Outstanding from earlier reviews:

• 🟡 #3976578124 — plugins/upsun/skills/upsun/SKILL.md:126: First-time users hit an unactionable step or get tunnelled into production. — First-time setup step 5 still reads upsun tunnel:open -p <PROJECT_ID> -e <PREVIEW_ID> with no preceding step that creates a preview (Git integration is still step 6).
• 🔵 #3976578130 — plugins/upsun/skills/upsun/SKILL.md:181: Recommended command fails or prompts in non-interactive agent runs. — Lines 181-182 still recommend upsun logs -p ... -e <PRODUCTION_ID> and upsun logs --tail -p ... -e <PREVIEW_ID> with no log-type argument, and the read-only exemption at line 240 still names only logs --tail.
• 🟡 #3976578134 — plugins/upsun/skills/upsun/SKILL.md:40: Skill authorizes unconfirmed production shell access for log reads. — Line 40 still grants production SSH log reads "without first attempting preview reproduction" while upsun ssh appears in neither the write-confirmation list nor the line-240 read-only exemption, and no scoping question is defined.
• 🔵 #3976578141 — plugins/upsun/skills/upsun/references/preview-environments.md:30: Ambiguous example invites a redundant destructive data overwrite. — The fenced block still lists the two upsun sync commands on consecutive lines with no comment marking them as alternatives, so a copy-paste runs a second destructive data sync.
• 🔵 #3976578148 — plugins/upsun/skills/upsun/references/preview-environments.md:46: Dead reference link sends the agent and reader nowhere. — The hooks link at preview-environments.md:46 still uses docs/configure-apps/hooks/hooks-comparison, unchanged by this push and unique in the repo.

Verification

• Every renamed anchor target #prefer-preview-environments-for-development matches the new SKILL.md heading text at line 34, and no stale -for-experiments anchor remains anywhere in the repo.
• The reference-file link text rename to "Development in previews" left the relative path references/preview-environments.md and its sub-anchors #test-a-database-migration / #compare-performance intact, and both headings still exist in the file.
• No eval case or repository config references the old "Preview experiments" title, so the rename breaks no automated check (grep over evals/** finds no preview/tunnel assertions).

This pass adds no tests; the only automated check is .github/workflows/run-evals.yml (deepeval on PRs), and no eval case references the preview-environment guidance, so the reworded sections are unverified beyond the manual skill-validator/git diff --check run claimed in the description.

Review details

• Commits: ed8c805..2a109d1
• Model: claude-opus-5

---

Review 2 of 10 for this pull request · View the full run

[upsun-dispatch]

Note

Reviewed — No new blocking findings · 🔵 1 minor point · 2 still open

🔁 Incremental · 2 files reviewed

🔵 Minor point

• plugins/upsun/skills/upsun/SKILL.md:40 — The revised container-log exception tells the agent to "Execute a specific read-only log command scoped to the relevant logs and time window", but the log command the skill itself recommends (upsun logs app --lines 100 ..., line 181) has no time-window selector — the CLI's log command offers only a line count and --tail. An agent taking the instruction literally either invents a non-existent flag (--since, --from) and gets an unknown-option error, or falls back to an SSH shell read that the same bullet says is not authorized as an interactive session. Naming the achievable scoping (log type, container, line count, or a bounded grep/sed over the file) would make the instruction executable.

Outstanding from earlier reviews:

• 🟡 #3976578134 — plugins/upsun/skills/upsun/SKILL.md:40: Skill authorizes unconfirmed production shell access for log reads. — Partially addressed: line 40 now forbids an unrestricted interactive production shell, but upsun ssh still appears in neither the write-confirmation list nor the read-only exemption (line 241 names only list/info/get/logs), so scoped production SSH log reads still have no defined confirmation or scoping check.
• 🔵 #3976578148 — plugins/upsun/skills/upsun/references/preview-environments.md:52: Dead reference link sends the agent and reader nowhere. — Unchanged: line 52 still links developer.upsun.com/docs/configure-apps/hooks/hooks-comparison, the only configure-apps path in the repository.

Verification

• The new claim "The bundled MCP configuration disables writes" matches plugins/upsun/.mcp.json, which sets header enable-write: "false".
• The sync examples are now two separate fenced blocks introduced by "Choose one of these alternatives" / "Or refresh code and data together", so copying the block no longer runs two data syncs.
• Step 5 of First-time setup now instructs selecting or creating a preview (linking to Branch / Merge) and verifying its ID before upsun tunnel:open.
• The intra-file anchors used by the changed lines resolve: #prefer-preview-environments-for-development, #branch--merge-feature-environments, #test-a-database-migration, #compare-performance.
• Both production/preview log examples now pass an explicit log type (app) and the read-only exemption line was widened from logs --tail to logs, so the recommended form is covered.

No automated check covers this documentation change: the only CI job is .github/workflows/run-evals.yml (Run Evaluations), whose sole test (evals/test_login.py) exercises upsun login status and asserts nothing about preview/log/sync guidance; the author states the hosted evals were not run locally. The skill validator and git diff --check mentioned in the description are not present in the repository's workflows.

Review details

• Commits: 2a109d1..d870a8a
• Model: claude-opus-5

---

Review 3 of 10 for this pull request · View the full run

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 2 minor points

🔁 Incremental · 1 file reviewed

🔵 Minor points

• plugins/upsun/skills/upsun/SKILL.md:118 — Step 4 still tells the agent to run upsun log --tail app as the routine post-push check. --tail streams continuously and never exits, so the agent's Bash call blocks until the tool timeout and returns no usable output — and the new frontmatter entry Bash(upsun log *) (line 4) auto-approves it, so nothing prompts the developer first. It also contradicts the guidance added at line 183 ("Bound reads with --lines; add --tail only when continuous streaming is needed"). A bounded read such as upsun log app --lines 100 is what this step needs.
• plugins/upsun/skills/upsun/SKILL.md:4 — The frontmatter now allows upsun log * / upsun environment:logs * / upsun env:logs *, but the manually-installed permission set documented in README.md (line 87) and used by evals/test_login.py (line 15) still lists only Bash(upsun logs:*), a namespace that matches none of the commands the skill now prescribes (upsun log app --lines 100). Developers following the README's manual-install instructions get a permission prompt for the skill's primary log command, and the eval harness runs the skill without the log permission it now relies on.

Verification

• No occurrence of the old upsun logs spelling remains anywhere under plugins/ after the rename; the two remaining hits are README.md and evals/test_login.py.
• The new allowed-tools patterns Bash(upsun log *) and Bash(upsun env:logs *) prefix-match every log invocation the skill body writes (upsun log app --lines 100 ...).
• The in-document anchor #prefer-preview-environments-for-development used at lines 176, 181 and 241 matches the heading added at line 34.
• The reference anchors references/preview-environments.md#test-a-database-migration and #compare-performance both resolve to headings present in that file.
• The sync block in preview-environments.md now separates sync data and sync code data into two fenced blocks introduced as alternatives, so copying one does not run both.

Nothing in the diff is test-covered: the only automated check is the run-evals GitHub Actions workflow (deepeval/Claude Code), whose sole test case (evals/test_login.py::test_upsun_login) exercises login and never touches log commands or preview guidance; the PR states hosted LLM evals were not run locally.

Review details

• Commits: d870a8a..c914900
• Model: claude-opus-5

---

Review 4 of 10 for this pull request · View the full run

[ralt]

Addressed both minor points from Dispatch review 4 in the latest pushed commit:

• The routine post-push check now uses upsun log app --lines 100, so it finishes instead of streaming continuously.
• README manual-install permissions and the eval harness now use the same upsun log *, upsun environment:logs *, and upsun env:logs * patterns as the skill.

Validation: skill validator, Python syntax parsing, README JSON parsing, and git diff --check pass. Hosted LLM evaluations were not run.

These findings were in the review summary rather than separate threads. All existing review threads are already resolved.

[upsun-dispatch]

📋 PR Summary

The skill now steers development, testing, and review work toward Upsun preview environments rather than production, adding a dedicated reference on external-service isolation, parent-to-child sync, migration testing, resource comparison, and cleanup. Alongside this, the documented CLI permission set and log commands are corrected to the real command names (upsun log, upsun environment:logs/env:logs) instead of upsun logs:*.

Changes

Layer / File(s)	Summary
Skill guidance
plugins/upsun/skills/upsun/SKILL.md	Reworks guidance to make preview environments the default place to develop, debug, and test, keeping scoped production log reads available; also fixes the post-push log example to upsun log app --lines 100.
plugins/upsun/skills/upsun/references/preview-environments.md	New reference covering external-service isolation, parent-to-child data sync, repeatable migration tests, resource comparisons, and environment cleanup/retention for review.
CLI permission allowlists
README.md	Replaces the Bash(upsun logs:*) permission entry with upsun log *, upsun environment:logs *, and upsun env:logs *.
evals/test_login.py	Applies the same allowlist update to the permissions used by the eval harness.

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 1 minor point

🔁 Incremental · 3 files reviewed

🔵 Minor point

• README.md:87 — The replacement allow-list entry Bash(upsun log *) (mirrored in evals/test_login.py:15 and in the skill's allowed-tools frontmatter) pre-approves every invocation of the logs alias, including the streaming form upsun log app --tail that SKILL.md still permits ("add --tail only when continuous streaming is needed"). The old Bash(upsun logs:*) entry matched no real CLI command, so log reads previously fell through to a permission prompt. With the new pattern an agent that chooses --tail runs a command that never returns and blocks the session until the host tool's timeout, with no confirmation step. Scoping the grant to bounded reads (or keeping --tail out of the pre-approved set) avoids that.

Verification

• The upsun log app --lines 100 example on SKILL.md:118 matches the documented alias and options described at SKILL.md:183 (log = environment:logs, positional type, --lines).
• The three new allow-list entries cover every logs invocation the skill instructs (upsun log app --lines 100 -p … -e … at SKILL.md:181-182).
• README.md and evals/test_login.py now carry identical permission lists, so the documented and eval-time grants stay in sync.
• No remaining reference to the removed upsun logs:* pattern exists anywhere in the repo (grep over all files).

No test covers skill or README content; the .github/workflows/run-evals.yml job (deepeval test run .) executes evals/test_login.py, whose --allowedTools list this diff edits, but that test only asserts login-status wording and passes --dangerously-skip-permissions, so the permission-pattern change is effectively unverified.

Review details

• Commits: c914900..bc11cac
• Model: claude-opus-5

---

Review 5 of 10 for this pull request · View the full run

[ralt]

Regarding Dispatch review 5's minor point about the log allowlist: allowing --tail is intentional when continuous streaming is requested. Routine post-deploy and debugging examples use bounded reads (upsun log app --lines 100), and the skill explicitly reserves --tail for cases where streaming is needed. We are keeping the log permission patterns as written; no code change is needed for this finding.