Skip to content

docs(architecture): update env delivery to reflect current schema - #2861

Closed
ralphbean wants to merge 1 commit into
mainfrom
docs/architecture-env-schema-update
Closed

docs(architecture): update env delivery to reflect current schema#2861
ralphbean wants to merge 1 commit into
mainfrom
docs/architecture-env-schema-update

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Summary

  • Replace stale runner_env references in docs/architecture.md with the current env.runner / env.sandbox terminology
  • Describe the unified env scheme as the current state rather than a deprecation in progress

Follows up on #2756.

Test plan

  • Confirm no remaining runner_env references in docs/architecture.md

🤖 Generated with Claude Code

Replace stale runner_env references with the current env.runner /
env.sandbox terminology (ADR 0055). The deprecation period is over;
describe the scheme as-is rather than as a migration in progress.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@ralphbean
ralphbean requested a review from a team as a code owner July 1, 2026 18:23
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Docs: align architecture env terminology with current env.runner/env.sandbox schema

📝 Documentation 🕐 Less than 10 minutes

Grey Divider

AI Description

• Replace stale runner_env references with env.runner / env.sandbox terminology.
• Describe unified env delivery as the current state, not a deprecation.
Diagram

graph TD
A["docs/architecture.md"] --> B["Env delivery section"] --> C["ADR 0055: Unified env delivery"]
B --> D["Harness YAML: env.runner / env.sandbox"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add a CI/docs lint to forbid `runner_env` references
  • ➕ Prevents future documentation regressions back to deprecated terminology
  • ➕ Catches stale references outside this specific doc file
  • ➖ Adds CI complexity for a one-time terminology cleanup
  • ➖ May require maintaining an allowlist for historical/quoted references

Recommendation: The PR’s direct documentation update is the right minimal fix for correctness today. If regressions have been common, consider a follow-up lightweight grep-based CI check to prevent reintroducing runner_env references.

Files changed (1) +4 / -4

Documentation (1) +4 / -4
architecture.mdUpdate env delivery terminology to env.runner/env.sandbox +4/-4

Update env delivery terminology to env.runner/env.sandbox

• Replaces 'runner_env' wording with the current unified 'env:' scheme terminology. Rephrases the env-delivery bullet to describe 'env.runner' and 'env.sandbox' as the established approach (per ADR 0055), not an ongoing deprecation/migration.

docs/architecture.md

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

Site preview

Preview: https://cf6d15de-site.fullsend-ai.workers.dev

Commit: 1ed96a2b7315e683c77a0acdeeb8ca6927b4645d

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 1, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:26 PM UTC · Completed 6:34 PM UTC
Commit: 1ed96a2 · View workflow run →

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@fullsend-ai-review

Copy link
Copy Markdown

Looks good to me


Labels: Docs-only change updating architecture.md terminology.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge component/docs User-facing documentation labels Jul 1, 2026
Comment thread docs/architecture.md
deprecated ([ADR 0055](ADRs/0055-unified-env-var-delivery.md), amending
sub-maps delivers environment variables. `env.runner` configures host-side
scripts; `env.sandbox` generates the sandbox `.env` file at bootstrap
([ADR 0055](ADRs/0055-unified-env-var-delivery.md), amending

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW — Removing deprecation language may be premature

runner_env is still functional: scaffold harnesses (code.yaml, review.yaml, retro.yaml) still use it, the Go struct still defines RunnerEnv, and lint.go emits deprecation warnings at runtime. ADR 0055 Phase 2–3 are not yet complete.

Consider a brief nod to the transition, e.g.:

a single env: key with runner and sandbox sub-maps delivers environment variables (superseding the deprecated runner_env)

This is a judgment call — the PR is correct as-is if the intent is to document the target state rather than the transitional state.

@ralphbean

Copy link
Copy Markdown
Member Author

Wayne's right — runner_env is still in active use and the migration isn't done. The text on main already accurately describes the transitional state. Closing this as premature; we can revisit once ADR 0055 Phase 2–3 are complete.

@ralphbean ralphbean closed this Jul 2, 2026
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 2, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 12:02 PM UTC · Completed 12:08 PM UTC
Commit: 1ed96a2 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #2861 — docs(architecture): update env delivery to reflect current schema

Timeline

  1. 2026-07-01 18:23 UTCralphbean opens PR #2861, a docs-only change (4 additions, 4 deletions) to docs/architecture.md. The change replaces runner_env references with env.runner/env.sandbox terminology and removes deprecation language, describing the unified env scheme as the current state.
  2. 2026-07-01 18:26–18:34 UTC — Review agent runs (32 tool calls, ~7 min). Dispatches 5 sub-agents: correctness, style-conventions, intent-coherence, docs-currency, and challenger.
  3. 2026-07-01 18:34 UTC — Review agent approves with "Looks good to me" and applies ready-for-merge + component/docs labels.
  4. 2026-07-01 20:53 UTC — Human reviewer waynesun09 posts an inline comment flagging that removing deprecation language is premature: runner_env is still functional in scaffold harnesses, the Go struct still defines RunnerEnv, and lint.go still emits deprecation warnings. ADR 0055 Phase 2–3 are not yet complete.
  5. 2026-07-02 12:00 UTCralphbean agrees with the human review and closes the PR without merging.

Analysis

Review quality gap: The review agent approved a PR that described the target state of an incomplete migration rather than the current transitional state. The human reviewer caught this by cross-referencing the codebase (harness YAML files, Go structs, linter warnings) — exactly the kind of verification the review agent's docs-currency and correctness sub-agents should have performed but did not.

Impact: Moderate. The ready-for-merge label was applied to a factually inaccurate docs change. Had the human not intervened, this could have been merged, making the documentation inconsistent with actual behavior.

Existing coverage

This pattern is well-recognized in the backlog. The following open issues already propose the improvements needed to prevent this class of miss:

  • #2199 — "Review agent should fact-check technical claims in documentation PRs" — directly covers verifying that docs claims match reality.
  • #2172 — "Review agent: cross-reference external identifiers in docs against codebase" — covers the specific mechanism (grepping for runner_env to verify it's actually removed).
  • #2676 — "Review agent should detect incomplete mechanical migrations across docs and examples" — covers the migration-completeness verification angle.
  • #1772 — "Review agent should defer ready-for-merge label on documentation-only PRs until human review" — covers the autonomy/labeling question.

Implementing any of #2199, #2172, or #2676 would likely have prevented this specific miss. No new proposals are warranted — this retro serves as additional evidence supporting those existing issues.

Autonomy assessment

The review agent's docs-currency and correctness sub-agents treated this as a straightforward terminology update without verifying the underlying migration status. Until the capabilities described in #2199/#2172 are implemented, the review agent should not be granted full autonomy on docs PRs that describe feature deprecation or migration status. The existing issue #1772 (defer ready-for-merge on docs PRs) would be a reasonable interim guardrail.

@github-actions
github-actions Bot deleted the docs/architecture-env-schema-update branch August 2, 2026 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/docs User-facing documentation ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants