docs(agents): correct the runOnce key rule this repo hands contributors - #3388
Conversation
…he broken half
rhdh-e2e-test-utils#149 fixed this in the library's own guide, but the file
someone writing an overlay spec actually reads still said:
Prefix with workspace name: "tech-radar-setup", "argocd-deploy"
That is the advice that produced #3318. Both examples above it used a literal
key around configure() and deploy(), so the two together read as an
endorsement of exactly the shape that breaks.
The rule now says why the "and projects" half matters — the flag directory is
keyed on the runner PID alone, so nothing in the path comes from the project —
and gives the fix as `${key}-${rhdh.deploymentConfig.namespace}`, which is what
deploy() has always done internally and why deploy() was never affected. Both
examples carry it.
A literal key stays correct for setup that is genuinely shared; bulk-import has
one of each on purpose, so the contrast is named rather than left as "avoid
literals". And nesting is marked as no rescue: a project-shared outer key skips
before deploy() is reached.
RHIDP-16456.
|
|
🤖 Finished Review · ✅ Success · Started 1:07 PM UTC · Completed 1:11 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $0.72 |
|
/publish |
|
PR action ( |
Review — approveDocumentation-only change to What was reviewedCorrectness of the technical explanation. The PR adds the const flagDir = path.join(os.tmpdir(), `playwright-once-${process.ppid}`);
const flagFile = path.join(flagDir, `${key}.done`);This matches the mechanism described in #3318 and #3375. The key insight — that nothing in the flag path comes from the Playwright project, so a literal key is shared across projects running the same spec — is accurate and well-motivated. Alignment with existing code. Verified against actual
Updated examples. Both code examples now use template literals with namespace suffix ( Nesting clarification. The updated nesting bullet correctly notes that a project-shared outer key prevents Impact scope. FindingsNo blocking findings. The change is factually accurate, well-scoped, and aligned with existing code patterns. 🤖 Reviewed with fullsend
|
fullsend-ai-review
Bot
added
the
requires-manual-review
github-actions
Bot
added
the
non-workspace-changes
|
🤖 Finished Retro · ✅ Success · Started 1:17 PM UTC · Completed 1:27 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.18 |
Retro: PR #3388 — docs(agents): correct the runOnce key ruleVerdict: clean workflow, no proposals. This was a docs-only PR by gustavolira correcting the Timeline
Review qualityThe review agent performed thorough verification for a docs PR:
No false positives, no missed issues in the content itself. The review agent's analysis was substantive — more detailed than the human review (author self-merged without a separate reviewer). Agents repoDiscovered from run 32851389850: agent definitions, skills, and harness configs resolved from Issue #3333 remains openPR #3388 implements exactly what issue #3333 (retro-filed, labeled Closed-loop successThis PR completes a full retro-to-resolution cycle: retro agent analyzed PR #3318 → filed issue #3333 → human implemented the fix in PR #3388. The retro's original finding (that the |
1 participant
One file, 19 lines. RHIDP-16456.
rhdh-e2e-test-utils#149 fixed this in the library's own guide and merged. But the file someone writing an overlay spec actually reads — this repo's
AGENTS.md— still says:That is the advice that produced #3318. Worse, both examples above it wrap
configure()anddeploy()in a literal key, so rule and examples together read as an endorsement of the shape that breaks.What changes
The rule now explains why the "and projects" half matters, rather than only asserting it:
Nothing in that path comes from the project. One spec in two projects — which is what adding an
-app-nextlane does — and the first project's setup satisfies the second, which then deploys nothing and fails much later on a missing element.The fix is stated as
`${key}-${rhdh.deploymentConfig.namespace}`, which is whatdeploy()does internally and whydeploy()was never affected. Both examples carry it now.A literal key stays correct for setup that is genuinely shared —
bulk-importhas one of each on purpose, so the contrast is named rather than reduced to "avoid literals". Nesting is marked as no rescue: a project-shared outer key skips beforedeploy()is reached, so its internal protection never gets a say.Scope
Documentation only, no code. The static check that would have enforced this was #3375, closed — it flagged nothing real and produced a false positive, so the convention is carried by the docs, which is where the reviewer said it belonged.
🤖 Generated with Claude Code