Skip to content

Commit 65ad3c5

Browse files
committed
Fix review nits: valid workflow JS, screenshot paths, doc accuracy
- Wrap both .claude/workflows/*.js procedural bodies in an exported async run() so they pass `node --check` (top-level return/await were illegal in the ES modules); schema/meta consts stay module-level. - Clarify Behat screenshot locations in ci-log-analyzer / review-app-triage: Behat writes to features/, CI moves them to web/screenshots/ on the review URL. - CLAUDE.md: correct the profiling-extensions row (xdebug toggles on/off/status; blackfire/newrelic only enable). - observability.md: mark Tier 2 OTel files as not-yet-present (land with PR #466). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NG6zFnH8rBuEykvqFBeC8X
1 parent de46c40 commit 65ad3c5

6 files changed

Lines changed: 70 additions & 60 deletions

File tree

.claude/agents/ci-log-analyzer.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ Sources you can read:
1717
Method:
1818
1. Identify the failing **stage/job** and its underlying `make` target.
1919
2. Find the **first** error (not the cascade after it). For Behat, read the JUnit failure message and
20-
the matching screenshot name under `web/screenshots/`.
20+
the matching screenshot: Behat writes PNGs to `features/` (FailAid `screenshot.directory` in
21+
`behat.default.yml`); in CI the `test:behat` job moves them to `web/screenshots/` and serves them on
22+
the review URL.
2123
3. Correlate with recent changes (`git log --oneline -15`, `git diff` against last green) to point at a
2224
suspect file/commit.
2325
4. Note environment-specific causes: SAPI mismatch (Unit vs FrankenPHP), DB engine, basic-auth on the

.claude/skills/review-app-triage/SKILL.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@ project's own validation targets — see [docs/ci-pipeline.md](../../../docs/ci-
1919
[ci-pipeline.md](../../../docs/ci-pipeline.md#job--make-target--script-map). The likely culprits:
2020
- `build:*` red → install failed: re-run `make all_ci` (or `make all` locally) and read the first
2121
error; usually composer resolution or `drush si`.
22-
- `test:behat` red → read the **JUnit** artifact (`junit/*.xml`) and the screenshots under
23-
`web/screenshots/`. Reproduce locally with `make behat`.
22+
- `test:behat` red → read the **JUnit** artifact (`junit/*.xml`) and the screenshots (Behat writes
23+
them to `features/`; in CI they are moved to `web/screenshots/` and served on the review URL).
24+
Reproduce locally with `make behat`.
2425
- `test:cinsp` / `test:upgradestatus` / `test:statusreport` / `test:watchdog` red → run the same
2526
target locally (`make cinsp`, `make upgradestatusval`, …); each prints the offending file/config/log.
2627
- `test:lighthouse` red → a category dropped below the `lighthouserc.yml` threshold; read

.claude/workflows/drupal-upgrade.workflow.js

Lines changed: 42 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -39,24 +39,26 @@ const VERDICT = {
3939
},
4040
}
4141

42-
// --- Gate: is D11 unblocked? ---
43-
phase('Gate')
44-
const gate = await agent(
45-
`Determine whether a Drupal 11 upgrade is currently unblocked for the skilld-docker-container repo.
42+
// The harness invokes this with the workflow context (phase/agent/log/budget helpers).
43+
export async function run({ phase, agent, log, budget }) {
44+
// --- Gate: is D11 unblocked? ---
45+
phase('Gate')
46+
const gate = await agent(
47+
`Determine whether a Drupal 11 upgrade is currently unblocked for the skilld-docker-container repo.
4648
Run \`composer show skilldlabs/druxxy --all\` (the default install profile) and check whether any
4749
release allows drupal/core ^11. Also skim composer.json require/require-dev for other obvious D11
4850
blockers. Return blocked=true if druxxy (or core) cannot resolve on ^11, with a one-line reason and,
4951
if blocked, a workaround (e.g. temporarily set PROFILE_NAME=standard on the throwaway branch).`,
50-
{ phase: 'Gate', schema: GATE },
51-
)
52-
if (gate?.blocked) {
53-
log(`D11 gate BLOCKED: ${gate.reason}. Proceeding on the throwaway branch with workaround: ${gate.workaround || 'PROFILE_NAME=standard'}.`)
54-
}
52+
{ phase: 'Gate', schema: GATE },
53+
)
54+
if (gate?.blocked) {
55+
log(`D11 gate BLOCKED: ${gate.reason}. Proceeding on the throwaway branch with workaround: ${gate.workaround || 'PROFILE_NAME=standard'}.`)
56+
}
5557

56-
// --- Apply the transitional changes in an isolated worktree ---
57-
phase('Apply')
58-
const apply = await agent(
59-
`In an isolated git worktree, apply the TRANSITIONAL Drupal 11 changes from docs/upgrading.md:
58+
// --- Apply the transitional changes in an isolated worktree ---
59+
phase('Apply')
60+
const apply = await agent(
61+
`In an isolated git worktree, apply the TRANSITIONAL Drupal 11 changes from docs/upgrading.md:
6062
1. composer.json: set drupal/core-composer-scaffold and drupal/core-vendor-hardening to "^10.3.1 || ^11";
6163
bump skilldlabs/druxxy to a D11-capable release IF ${gate?.blocked ? 'one exists (it may not — if blocked, leave druxxy and instead set PROFILE_NAME=standard in .env.default for validation only)' : 'available'}.
6264
2. Remove drupal/ckeditor, drupal/color, drupal/seven. git grep for stragglers; switch admin theme to claro.
@@ -65,37 +67,38 @@ const apply = await agent(
6567
4. Re-verify the drupal/default_content patch still applies; drop it if upstreamed.
6668
5. Run \`composer update --with-all-dependencies\` (via the php container) and report whether it resolved.
6769
Make minimal, reviewable edits. Return composerResolved, a summary, the removed modules, and any open problems.`,
68-
{ phase: 'Apply', schema: APPLY, isolation: 'worktree' },
69-
)
70-
log(`Apply: ${apply?.summary || 'no result'}`)
70+
{ phase: 'Apply', schema: APPLY, isolation: 'worktree' },
71+
)
72+
log(`Apply: ${apply?.summary || 'no result'}`)
7173

72-
// --- Validate and iterate to ready ---
73-
phase('Validate')
74-
let verdict = null
75-
const MAX_ITERS = budget.total ? 6 : 3
76-
for (let i = 1; i <= MAX_ITERS; i++) {
77-
verdict = await agent(
78-
`Validate Drupal upgrade-readiness of the current worktree. Run \`make upgradestatusval\` (authority),
74+
// --- Validate and iterate to ready ---
75+
phase('Validate')
76+
let verdict = null
77+
const MAX_ITERS = budget.total ? 6 : 3
78+
for (let i = 1; i <= MAX_ITERS; i++) {
79+
verdict = await agent(
80+
`Validate Drupal upgrade-readiness of the current worktree. Run \`make upgradestatusval\` (authority),
7981
\`make drupalrectorval\`, and \`make cinsp\`. Report ready=true only if upgrade_status prints no FILE:
8082
lines. List remaining blockers (custom-code deprecations, by file) and which are rector-auto-fixable.`,
81-
{ phase: 'Validate', label: `validate#${i}`, schema: VERDICT, agentType: 'upgrade-validator' },
82-
)
83-
if (verdict?.ready) { log(`Ready after ${i} iteration(s).`); break }
84-
if (!verdict?.blockers?.length) { log('No blockers reported but not marked ready — stopping to avoid a loop.'); break }
85-
log(`Iteration ${i}: ${verdict.blockers.length} blocker(s). Attempting fixes.`)
86-
await agent(
87-
`Fix these Drupal 11 deprecations in custom code only (web/modules/custom, web/themes/custom):
83+
{ phase: 'Validate', label: `validate#${i}`, schema: VERDICT, agentType: 'upgrade-validator' },
84+
)
85+
if (verdict?.ready) { log(`Ready after ${i} iteration(s).`); break }
86+
if (!verdict?.blockers?.length) { log('No blockers reported but not marked ready — stopping to avoid a loop.'); break }
87+
log(`Iteration ${i}: ${verdict.blockers.length} blocker(s). Attempting fixes.`)
88+
await agent(
89+
`Fix these Drupal 11 deprecations in custom code only (web/modules/custom, web/themes/custom):
8890
${verdict.blockers.map((b, n) => `${n + 1}. ${b}`).join('\n')}
8991
Prefer \`vendor/bin/rector process web/modules/custom web/themes/custom\` for the auto-fixable ones,
9092
then hand-edit the rest. Do not touch contrib/core. Keep edits minimal.`,
91-
{ phase: 'Validate', label: `fix#${i}`, isolation: 'worktree' },
92-
)
93-
}
93+
{ phase: 'Validate', label: `fix#${i}`, isolation: 'worktree' },
94+
)
95+
}
9496

95-
return {
96-
gate,
97-
apply,
98-
finalVerdict: verdict,
99-
done: !!verdict?.ready,
100-
note: 'Definition of done is a green D11 review app — open an MR and run build:review + the test:* jobs.',
97+
return {
98+
gate,
99+
apply,
100+
finalVerdict: verdict,
101+
done: !!verdict?.ready,
102+
note: 'Definition of done is a green D11 review app — open an MR and run build:review + the test:* jobs.',
103+
}
101104
}

.claude/workflows/newrelic-bump.workflow.js

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,24 @@ const RESULT = {
2626
},
2727
}
2828

29-
phase('Find')
30-
const latest = await agent(
31-
`Find the latest NewRelic PHP agent release version. Read the current pin from
29+
// The harness invokes this with the workflow context (phase/agent/log helpers).
30+
export async function run({ phase, agent, log }) {
31+
phase('Find')
32+
const latest = await agent(
33+
`Find the latest NewRelic PHP agent release version. Read the current pin from
3234
scripts/makefile/newrelic.sh (the NEW_RELIC_AGENT_VERSION default). Determine the newest available
3335
release version and its release-notes URL. Return version, releaseUrl, and current.`,
34-
{ phase: 'Find', schema: LATEST },
35-
)
36-
log(`NewRelic agent: current ${latest?.current || '?'} → latest ${latest?.version || '?'}`)
36+
{ phase: 'Find', schema: LATEST },
37+
)
38+
log(`NewRelic agent: current ${latest?.current || '?'} → latest ${latest?.version || '?'}`)
3739

38-
if (latest && latest.version && latest.version === latest.current) {
39-
return { bumped: false, summary: `Already at the latest agent version (${latest.version}).` }
40-
}
40+
if (latest && latest.version && latest.version === latest.current) {
41+
return { bumped: false, summary: `Already at the latest agent version (${latest.version}).` }
42+
}
4143

42-
phase('Bump')
43-
const result = await agent(
44-
`Bump the NewRelic PHP agent to ${latest?.version}.
44+
phase('Bump')
45+
const result = await agent(
46+
`Bump the NewRelic PHP agent to ${latest?.version}.
4547
1. Edit the version string in scripts/makefile/newrelic.sh (and any matching version in .env.default or
4648
docker config if present). Make a minimal diff.
4749
2. Run \`composer validate\`. If a stack is up, run \`make newrelic reload\` and confirm the agent loads
@@ -50,7 +52,8 @@ const result = await agent(
5052
3. Prepare a commit message in the repo's existing convention: "NR: <release-notes-url>"
5153
(see \`git log --oneline | grep '^... NR:'\`). Do NOT commit unless asked.
5254
Return bumped, a summary, and the proposed commitMessage.`,
53-
{ phase: 'Bump', schema: RESULT, agentType: 'dep-bumper' },
54-
)
55+
{ phase: 'Bump', schema: RESULT, agentType: 'dep-bumper' },
56+
)
5557

56-
return { ...result, from: latest?.current, to: latest?.version, releaseUrl: latest?.releaseUrl }
58+
return { ...result, from: latest?.current, to: latest?.version, releaseUrl: latest?.releaseUrl }
59+
}

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ and `make xdebug on` forward extra tokens to the wrapped command.
4040
| `make front` / `make lint` / `make storybook` | Theme build / lint+fix / Storybook (need `THEME_NAME`) |
4141
| `make tests` | Run the full validation + test suite (see below) |
4242
| `make behat` | Run Behat browser tests |
43-
| `make xdebug on\|off\|status`, `make blackfire`, `make newrelic` | Toggle profiling/debug PHP extensions |
43+
| `make xdebug on\|off\|status` / `make blackfire` / `make newrelic` | Profiling/debug PHP extensions`xdebug` toggles on/off/status; `blackfire` and `newrelic` enable only (`newrelic` needs `NEW_RELIC_LICENSE_KEY`) |
4444

4545
### Tests & validations
4646

docs/observability.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,10 @@ Unit's status/health route as a liveness probe in the override file's `php` serv
2828
## Tier 2 — Traces (opt-in OpenTelemetry)
2929

3030
Distributed tracing is prototyped in **PR #466** (`otel` branch). Rather than reinvent it, the plan is
31-
to **rebase and finish that PR**, then gate it behind a flag. What it wires up:
31+
to **rebase and finish that PR**, then gate it behind a flag. What it wires up (none of these files/
32+
settings exist on this branch yet — they land with PR #466):
3233

33-
| Piece | Where |
34+
| Piece | Where (added by PR #466) |
3435
| --- | --- |
3536
| Tempo (trace store) + Grafana (UI) services | `docker/docker-compose.override.yml.default` |
3637
| Tempo config | `docker/tempo.yml` |

0 commit comments

Comments
 (0)