feat(migration): Dispatcher config conversion for AEMaaCS — Branch E - #309
Open
Himanich wants to merge 34 commits into
Open
feat(migration): Dispatcher config conversion for AEMaaCS — Branch E #309Himanich wants to merge 34 commits into
Himanich wants to merge 34 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ted rules Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tion) + pinned tool package
Add references/dispatcher/context.md: the mode taxonomy (6 modes via dispatcher-inventory.js buildInventory), the auto-installed Adobe tool prerequisite (dispatcher-run.js ensureToolInstalled), the clean-git-tree pre-flight, the 6-phase flow (forward-linking Tasks 7-10's sibling refs), and per-mode automation honesty (standard=near-auto, flexible=assisted, v1=best-effort, already-cloud=report-only, not-dispatcher/unknown=stop).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…st-tool actions) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Documents Branch E phase 2 (PLAN + CONFIG-GEN): how the agent turns buildInventory() output into the dispatcher-converter tool's config.yaml, per mode (standard's ams.cfg vs. flexible/v1's onPremise.* fields), with a full worked example and the variablesToReplace/portsToMap shape gotchas. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The on-premise executor does fs.readFileSync(appendToVhosts) uncaught (README: "This can be a FILE that you want to append to every vhost file"). Passing inline directive text instead of a path crashes the whole conversion with ENOENT. Update the cfg-shape comment and the field table to say so explicitly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t-scan → inventory) Register an 11th runbook pattern `dispatcherConversion` (strategy `content-scan`, `bpaSlugs: []`) and wire the existing `runDispatcherScan` (dispatcher-inventory.js) as its content scanner, so the migration runbook detects convertible AMS/on-prem Dispatcher configs (Branch E). The scanner already conforms to the content-scan contract and skips already-cloud / non-dispatcher workspaces, so existing 10-pattern behavior is unchanged and non-dispatcher runs gain no noise. Also run the existing dispatcher-conversion.test.js under `npm test` and document the three dispatcher scripts (inventory / verify / run) in README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Weave a new Branch E (6-phase dispatcher conversion flow) into SKILL.md alongside Branches A-D: quick-start row, required-delegation block (beta- gated), when-to-use bullet, Step-0 runbook detection row, Step-1 routing, Step-3 targets note, and a description-frontmatter clause. Additive only; links target the six existing references/dispatcher/*.md files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ilently pass
Final whole-branch review found (and reproduction confirmed) that the
filter/ACL hard gate silently PASSED on the canonical AMS "standard"
layout: filters live in conf.dispatcher.d/filters/*_filters.any and are
only $include'd from the farm, so buildInventory counted 0 filter rules,
verifyOutput's baseline.filter>0 guard was false, and an emptied output
was waved through.
- Add countFilterRules(root) — counts inline farm /filter{} rules PLUS
standalone filter files (filters.any, *_filters.any, or a .any file in a
filters/ dir). Used by BOTH buildInventory (baseline) and verifyOutput
(output) so the two sides count identically (no false-pass, no false-fail).
- Regression test: AMS-standard $include layout -> ruleCounts.filter===3 and
an empty output -> verifyOutput.ok===false with filter-acl-loss/critical.
- SKILL.md Branch E phase 1: STOP also on 'unknown' (falls through to the
on-prem executor; the agent is the gate).
- output-verification.md + README.md: document the symmetric counter and the
residual $include-outside-filters/ blind spot.
- Minor cleanups: drop dead always-zero term in verifyOutput; surface
spawnSync error in runConverter stdout; refresh content-scan taxonomy
comments; guard null dispatcherAny + clean temp dir in the e2e test;
note Node>=18 test prereq in README (engines unchanged).
Full unit suite 20/20 (incl. new regression test).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… artifact) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ate count
The AEMaaCS SDK ships Adobe-managed immutable filter files
(default_filters.any and friends) in conf.dispatcher.d/filters/. Those
files live in a filters/ directory and end with _filters.any, so
countFilterRules counted them like customer ACLs. That let a populated
SDK default_filters.any mask an emptied custom filters.any: the output
count stayed non-zero (baseline custom 8, output custom 0 but default
12 -> 12 != 0, no acl-loss; 8 > 12 false, no regression), so the
critical filter-acl-loss gate in verifyOutput silently passed while the
customer's ACLs were dropped.
Exclude any standalone filter file whose basename starts with `default_`
from countFilterRules (in the shared counter, so both the buildInventory
baseline and the verifyOutput output apply it symmetrically). The count
is now custom-to-custom: AMS baselines have no default_*.any so nothing
changes there, and an emptied custom output scores 0 even when the SDK
default survives -> the gate fires. The exclusion is scoped to the
standalone-file scan branch only; the inline /filter{} term is
unchanged, and comment-stripping is preserved.
Tests: add a masking-regression test (empty custom + populated
default_ -> critical filter-acl-loss), a positive test (preserved
custom 3 + default_ 12 -> no false fail), and a baseline-symmetry test
(default_ at input excluded from ruleCounts.filter). Docs: mark the
default_ masking case RESOLVED in output-verification.md (keeping the
remaining out-of-filters/ include caveat) and refresh the
dispatcher-verify.js entry in scripts/README.md.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…on (Branch A) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ent to the dispatcher skill) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… router Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…'t mask dropped custom rewrites The conversion coverage report derived its rewrite Output count from buildInventory().ruleCounts.rewrite, which (unlike countFilterRules) applies no default_* exclusion. A surviving Adobe-managed default_rewrite.rules (SDK boilerplate the converter always adds) could therefore inflate the output count and mask dropped custom rewrites as "preserved" — the same masking class as the Phase-1 filter Critical. Report-only fix (no Phase-1 mutation, verifyOutput never imported/called): - Add countCustomRewrites(dir): counts RewriteRule/Redirect(Match)? across .rules + .vhost (glob aligned with dispatcher-verify.js), excluding default_*. - Override out.rewrite with the custom-only count when outputSrcDir is present. - Add a note under the coverage table that Output counts are custom-only. - When outputSrcDir is absent, render Output/Status as "not scanned" instead of a false DROPPED; update spec 5.2 renderReport signature to include outputSrcDir. Tests: masking regression (2 custom + 5 default -> Output 2 / partial(2/4), not 7/preserved), preserved case, and not-scanned case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…erage, quote tool-config YAML Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…(minor review fixes)
Six focused Minor fixes from senior review of the dispatcher-conversion skill:
- A: runConverter refuses non-convertible modes (CONVERTIBLE={standard,flexible,v1})
-> {code:null, "...not convertible..."} without spawning (defense-in-depth).
- B: converter spawnSync gets timeout:600000 + maxBuffer:64MiB so a big config's
verbose stdout can't ENOBUFS a successful run and a hung tool can't hang the caller.
- C: escape `|` in the CM-handoff table's dynamic name + path:line cells so a pipe
in a path can't break the Markdown row into a spurious column.
- D: strip full-line `#` comments before the ${VAR} scans in inventory cmVarCandidates
and crossboundary analyzeCrossBoundary (both Define + ${VAR} detection) so a
commented `# ${OLD}` no longer yields a phantom variable.
- E: rename localeDefined -> locallyDefined (cosmetic; no behavior change).
- F: also scan `*.vhost.tmpl` (templated vhost) in the crossboundary predicate.
Preserves the filter/ACL critical gate and the unified rewrite counter. Adds tests
for A (refusal, no target/), C (pipe-escape), D (comment-strip on both scanners), and
F (.vhost.tmpl capture). Full suite: 92 tests, 90 pass, 2 skipped (e2e self-skips).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…cloud runtime invariants Domain review (AEMaaCS dispatcher) fixes: - filter-acl-loss consequence is over-restrictive (default_filters.any deny-all present) vs wide-open (no deny-all) — not unconditionally 'wide-open'. - rewrite consolidation preserves dispatcher-side rules but TLS/host + large vanity-URL redirect maps belong at the CDN edge; reconcile with the rewrite-drop warning. - current-sdk-conventions names the validator-enforced runtime invariants (ServerAlias adobeaemcloud, reserved /system/probes/*, /ignoreUrlParams, core vhost defaults), delegating the authoritative contract to the dispatcher skill. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Himanich
requested review from
abhishekgarg18,
akankshajain18,
pkumargaddam and
rombert
as code owners
August 25, 2026 05:42
Contributor
Tessl Plugin Lint
|
…nore commented sections (PR review)
Addresses 5 human PR-review findings on the dispatcher-conversion skill:
1. [SECURITY] Broaden the rule-entry regex /[0-9]{3,4} → /[\w.-]+ in
countSectionRules and the countFilterRules standalone scan so non-4-digit /
non-numeric ACL labels (/01, /10001, /allow-html) are counted. Under-counting
let an emptied output pass the filter-acl-loss gate (fail-open); the broadened
regex only over-matches, which is the safe fail-closed direction.
2. findConfigRoots: once a dir is a config root, push and return without recursing
into its children — fixes the flexible-tree double-add [root, root/conf.d].
3. countSectionRules / countQuotedEntries: strip full-line # comments from the
whole file before running the section-opener regex so a commented `# /filter {`
can no longer inflate the baseline; drop the redundant inner comment filter.
4. dispatcher-verify: reword the rewrite-drop warning to name both benign causes
(.tmpl inflation / CDN edge move) instead of implying loss.
6. Add after-hook temp-dir cleanup to the conversion/report/crossboundary tests.
Adds Fix-1/2/3 unit tests. #5 (report perf) and #7 (portsToMap) left unchanged
per review (verified non-bugs).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rombert
reviewed
Aug 31, 2026
rombert
left a comment
rombert
left a comment
Member
There was a problem hiding this comment.
I think @abhishekgarg18 is best suited to review here. You should also see why PR checks failed.
CI (validate + tessl-review) failed on PR #309: 'description' must be 1-1024 characters, got 1047. The Branch E clause added during dispatcher conversion wiring pushed it over. Shortened that clause (no other content changed); new length 1008. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds Branch E — Dispatcher Conversion to the AEM Cloud Service migration skill: converts AMS / on-premise Apache + Dispatcher configurations to AEM as a Cloud Service. It is agent-driven and wraps Adobe's maintained @adobe/aem-cs-source-migration-dispatcher-converter as the deterministic engine — adding the detection, verification, judgment, and reporting layers the raw tool lacks — and hands off to the existing dispatcher skill rather than duplicating it.
Related Issue
Motivation and Context
How Has This Been Tested?
Unit suites + an env-gated real-config acceptance test
Screenshots (if appropriate):
Types of changes
Checklist: