Skip to content

Phase 4 Design Quality Review (round 1)

Date: 2026-05-12. Reviewer: author of the docs (Miguel Pinilla / Claude session email-integration-11), with explicit bias-mitigation disciplines stated below. A prior fresh-agent attempt produced a shallow hallucinatory output; this review supersedes it.

In-scope files (full review, listed by real worktree path):

  1. 4-runtime-platform-updates/design/analysis.md
  2. 4-runtime-platform-updates/design/requirements.md
  3. 4-runtime-platform-updates/design/specification.md
  4. 4-runtime-platform-updates/design/verification.md
  5. 4-runtime-platform-updates/design/exports.md
  6. 4-runtime-platform-updates/design/plan/task-plan.md
  7. email-server-key-encryption.md
  8. cross-cutting-design.md (§ 7a only)
  9. phases.md (Phase 4 deliverables block + Phase 5b open question)
  10. 5a-component-library-updates/pre-existing-decisions.md (B1 patch)
  11. 5b-email-module/pre-existing-decisions.md (DQ-R1-020 + DQ-R1-023 + transactional-only)

Source-of-truth / reference (read-only — flag drift only):

  • 4-runtime-platform-updates/goal.md — Phase 4 origin of intent.
  • decision-log.md — established decisions (DQ-001..013, DQ-R1-001..023).
  • goal.md, architecture-overview.md — project-level context.
  • cdk-infrastructure skill — CDK authoring conventions.
  • Templates under documentation/src/content/docs/about/templates/.

Author-bias mitigations applied (stated upfront for transparency):

  • Goal-derived and decision-log-derived constraint checklists extracted literally before opening any design file; findings graded against the literal text, not against remembered intent.
  • For every “this is fine because we decided X” instinct: if X isn’t visibly in the doc or in decision-log.md, flag as incompleteness.
  • Cross-document claims verified by grep -n, not recall.
  • Canonical sources (skills) opened at the moment a Deviation finding is drafted; the rule is captured literally before forming the verdict.
  • A Deviation finding I cannot ground in a canonical source is downgraded to Ambiguity or dropped.
  • Where author-familiarity blindness threatens objectivity most (CDK pattern compliance in specification.md / exports.md / verification.md), a focused fresh-agent slice may be run separately and its findings folded in.

Confirmed open during the review:

  • 4-runtime-platform-updates/goal.md (end-to-end, 163 lines).
  • decision-log.md (full DQ-R1-017..023 entries; surrounding entries via grep).
  • Workspace-root CLAUDE.md (AWS-impact discipline, CHANGELOG model, attribution rules; the agent-only file at the root of the Arda development workspace — not part of any individual repo).
  • Project-governance CLAUDE.md under projects/email-integration-worktrees/ (worktree map, branch model, stack-and-PR conventions specific to this project; also workspace-local, gitignored).
  • cdk-infrastructure skill (spot-opened for: three-interface pattern, validateProps/MultiError, publish(), -API- vs -I- export naming, CFN_IO_MARKER dual-emission, NoEcho-CFN-parameter pattern δ.1, tryGetContext layering, findOutputs assertion shape, Phase A imperative / Phase B declarative split).

(Skills opened on demand per finding; not full end-to-end reads.)

(Appended as the review proceeds. Final sort happens at § 6 closure.)

IDCategorySeverityLocationSummaryStatus
QR1-001InconsistencyHighanalysis.md §§ 4.2, 7.3, 12.4, 13.4; requirements.md REQ-PART-017/018; verification.md V-PART-017 title”Per-Infrastructure” DNS-records role contradicts goal.md success criterion #4 (“Each partition has two per-purpose IAM roles”) and specification.md L187 + L263 (single-zone scope per partition).Resolved 2026-05-12 — see § 4.QR1-001 Resolution
QR1-002InconsistencyMediumanalysis.md § 5.3 G-POSTMARK-2 (vs § 10.3 G-OP-2)“Created via Postmark Account API in amm.sh’s post-deploy step” contradicts § 10.3’s “Runs before the cdk deploy” and DQ-R1-022’s Phase A → Phase B sequencing. Same-document slip.Resolved 2026-05-12 — see § 4.QR1-002 Resolution
QR1-003DeviationMediumanalysis.md § 7.3 G-IAM-1; specification.md T-I1 (L92)Construct rename bundled into the byte-identity guard PR. DQ-R1-020 says to defer the rename. The override (preserve construct ID at call site) is technically sound but not flagged as an override in the decision-log.Resolved 2026-05-12 — see § 4.QR1-003 Resolution
QR1-005InconsistencyMediumanalysis.md § 4.3 G-DNS-5 (L90), § 10.3 G-OP-4 (L288) ↔ specification.md § 3 (L733)PR allocation drift: spec.md moves T-I7 and T-I9 into PR #1 (workspace); analysis.md still maps the corresponding gap-rows (G-DNS-5, G-OP-4) to per-partition groups G-B / G-C.Resolved 2026-05-12 — see § 4.QR1-005 Resolution
QR1-004InconsistencyLowanalysis.md § 16 (L570)“seven -API- exports” contradicts the six exports listed in § 4.3 / § 12 / exports.md § 1 / specification.md L265. Enumeration double-counts the Postmark-token SM-secret ARN.Resolved 2026-05-12 — see § 4.QR1-004 Resolution
QR1-006DeviationLowplan/task-plan.md § 1 (L32) and § 9 (L220)Persona reference suggests back-end-engineer (Kotlin persona) as alternative for TypeScript/CDK work; per feedback_ts_agent_type should be devops-engineer only (or front-end-engineer for pure TS packaging).Resolved 2026-05-12 — see § 4.QR1-006 Resolution

QR1-001 — DNS-records role: “per-Infrastructure” vs “per-partition”

Section titled “QR1-001 — DNS-records role: “per-Infrastructure” vs “per-partition””
  • Category: Inconsistency
  • Severity: High
  • Location: analysis.md § 4.2 (L78), § 7.3 G-IAM-3 (L194), § 12.4 (L385, L388), § 13.4 (L516); requirements.md REQ-PART-017 (L152–156) and REQ-PART-018 (L158–162); verification.md V-PART-017 title (L349) and the traceability-table row L57
  • Observation: Analysis.md repeatedly describes the DNS-records role as “per-Infrastructure” (one per Alpha account, scoped via allowedParentHostedZoneIds to “that Infrastructure’s partition mail sub-zones”). § 12.4 even says “C-Runtime-DNS-Writes (per Infrastructure — two Infrastructures = two completions)”. requirements.md REQ-PART-017 propagates the ambiguity verbatim — “one role instance per partition, or one role per Infrastructure covering both partitions — implementation choice resolved in specification.md” — and REQ-PART-018 inherits it. The actual design is per-partition: goal.md success criterion #4 says “Each partition has two per-purpose IAM roles, declared in the partition-email stack”; specification.md L187 declares emailDnsProvisioningRole: iam.IRole once per stack (each stack is per-partition); spec.md L263 sets allowedParentHostedZoneIds: [zone.hostedZoneId] — single-element array scoped to that partition’s zone only.
  • Rationale: If a reader takes analysis.md or requirements.md literally, they could build one role per Alpha account with trust + permissions covering both partitions in that account (e.g., one role in Alpha002 that both dev and stage pods can assume, scoped to both dev.ardamails.com and stage.ardamails.com). That breaks per-partition least-privilege isolation — a dev pod could write to stage.ardamails.com. The framing in requirements.md is doubly problematic: it presents a goal-fixed constraint as an “implementation choice”, as if the per-partition vs per-Infrastructure decision is still open. It is not.
  • References: goal.md § Success Criteria #4 + § Scope “Per-partition DNS-provisioning role”; decision-log.md DQ-R1-020 “in each partition”; specification.md L187 (Built interface) + L263 (allowedParentHostedZoneIds: [zone.hostedZoneId]).
  • Options:
    1. Rewrite analysis.md’s “per-Infrastructure” usages to “per-partition” (and “two Infrastructures = two completions” to “four partitions = four completions”); rewrite requirements.md REQ-PART-017/018 to state per-partition unambiguously without the “implementation choice” framing.
    2. Keep “per-Infrastructure” as a deliberate divergence if the design actually intends one role per Alpha account — but then update goal.md / spec.md to match, and add a least-privilege rationale (which would also need to argue against per-partition isolation).
  • Recommendation: Option 1. The IAM design that spec.md actually implements is per-partition, matching goal.md. The analysis.md + requirements.md wording is terminology slippage and should be corrected before implementation starts.
  • Resolution (2026-05-12): Option 1 applied. analysis.md rewritten across § 4.2 (L78), § 7.3 G-IAM-3 (L194 — added single-zone scope statement + least-privilege isolation rationale), § 12.4 (L385 → “per partition — four partitions = four completions”; L388 AWS-impact → “per partition for both roles”), § 13.4 (L516 — readiness criterion now reads “for each partition” + spells out trust-policy shape). requirements.md REQ-PART-017 title → “Per-partition DNS-records role instantiated”; body rewritten to state allowedParentHostedZoneIds: [zone.hostedZoneId] (single-element array, single-zone scope) and added explicit isolation rationale. REQ-PART-018 dropped the “or per Infrastructure if a single role serves both partitions” hedge. verification.md V-PART-017 title → “instantiated per partition”; traceability-table link anchor + REQ-PART-017 link anchor both updated to lowercase-hyphen slug. Cross-doc grep confirms no residual “per-Infrastructure” mentions in design package outside exports.md L91 (unrelated — amm.sh’s all-token expansion).

QR1-005 — PR allocation drift between analysis.md and specification.md

Section titled “QR1-005 — PR allocation drift between analysis.md and specification.md”
  • Category: Inconsistency
  • Severity: Medium
  • Location: analysis.md § 4.3 G-DNS-5 (L90), § 10.3 G-OP-4 (L288), § 12 Implementation Groups; specification.md § 3 PR table (L731–739)
  • Observation: spec.md moved T-I7 (reserved-words extension) and T-I9 (TypeScript helper extraction) from PR #2 into PR #1 with an explicit “Previously this task was bundled into PR #2; moving to PR #1 tightens scope per content-type” note. analysis.md was not updated — G-DNS-5 still mapped to G-B (per-partition); G-OP-4 still mapped to G-C (per-partition). The two documents disagreed on the per-partition vs workspace placement of these two gap-rows.
  • Rationale: A reader following analysis.md alone would expect reserved-words + helper extraction in the per-partition PRs; the spec.md PR table is authoritative. Drift makes the package self-contradictory and risks an implementer following the wrong doc.
  • References: specification.md § 3 (L731–739, L744 sequencing rules); analysis.md § 12.1 G-A, § 12.2 G-B, § 12.3 G-C.
  • Options:
    1. Update analysis.md to match spec.md (move G-DNS-5 + G-OP-4 from G-B / G-C into G-A; update DAG + edges to add a hard G-A → G-C dependency because G-C’s register-partition-mail-signature.ts now imports the helpers G-OP-4 lives in G-A).
    2. Revert spec.md’s move (put T-I7 + T-I9 back into PR #2).
  • Recommendation: Option 1. Spec.md’s scope-by-content-type framing is the right call (workspace refactors land together; per-partition PRs stay focused on the partition’s resources).
  • Resolution (2026-05-12): Option 1 applied. analysis.md § 4.3 G-DNS-5 (L90) — Group column changed G-BG-A; description gained “Workspace-scoped (no per-partition fan-out) — landed in PR #1 (G-A) alongside the other workspace refactors”. § 10.3 G-OP-4 (L288) — Group column changed G-CG-A; description gained “Workspace-scoped — landed in PR #1 (G-A) so the helpers are available when G-OP-2’s register-partition-mail-signature.ts lands in PR #2”. § 12.1 G-A’s Contains line extended to include G-DNS-5 + G-OP-4 (with workspace-scope annotations); test surface gained reserved-words-list assertion and tools/lib/*.test.ts coverage; acceptance criteria gained the corresponding bullet. § 12.2 G-B’s Contains line corrected: “G-DNS-1 through G-DNS-4 … G-DNS-5 is workspace-scoped and lands in G-A — see § 12.1”. § 12.3 G-C’s Contains line corrected: “G-OP-1, 2, 3 (G-OP-4 — TypeScript helper extraction — is workspace-scoped and lands in G-A)”. § 13.1 DAG and § 13.2 edges table gained a new hard edge G-A → G-C (“G-C’s register-partition-mail-signature.ts (G-OP-2) imports the tools/lib/* helpers extracted in G-A (G-OP-4)”); the PlantUML diagram source updated accordingly.

QR1-002 — Postmark API call ordering: pre-deploy vs post-deploy

Section titled “QR1-002 — Postmark API call ordering: pre-deploy vs post-deploy”
  • Category: Inconsistency
  • Severity: Medium
  • Location: analysis.md § 5.3 G-POSTMARK-2 (L125) vs § 10.3 G-OP-2 (L286) and § 13.3 (L490)
  • Observation: G-POSTMARK-2 says “Created via Postmark Account API in amm.sh’s post-deploy step”. G-OP-2 and § 13.3 correctly state the API call (Phase A) runs before cdk deploy (Phase B), because the DKIM values returned by Postmark are written into cdk.context.json which CDK reads at synth time. Same-document contradiction.
  • Rationale: A reader of § 5 alone could implement post-deploy ordering, which would mean the first cdk deploy lacks DKIM context values and the construct emits incomplete records. The Phase A → Phase B sequence is load-bearing per DQ-R1-022 and Phase 3’s corporate-cli precedent.
  • References: decision-log.md DQ-R1-022 Implementation route (L849–854); analysis.md § 10.3 G-OP-2 (the correct statement); analysis.md § 13.3 readiness preconditions (the correct statement).
  • Options:
    1. Reword G-POSTMARK-2 to “Created via Postmark Account API in amm.sh’s Phase A step, before the cdk deploy (per § 10.3 G-OP-2).”
    2. Delete the ordering claim from G-POSTMARK-2 and let § 10 own the sequencing.
  • Recommendation: Option 1. Make every section that mentions the Postmark API call name the phase (A) and the position (before cdk deploy) so isolated reads of any section don’t get the wrong order.
  • Resolution (2026-05-12): Option 1 applied. analysis.md § 5.3 G-POSTMARK-2 (L125) rewritten: “Created via Postmark Account API in amm.sh’s Phase A step (before cdk deploy) — the API call returns the DKIM selector / Return-Path target which tools/register-partition-mail-signature.ts writes into cdk.context.json, and cdk deploy (Phase B) reads that context at synth time. See § 10.3 G-OP-2 and § 13.3 for the operator-level sequencing.” Cross-doc grep confirms no remaining “post-deploy step” wording in analysis.md.

QR1-003 — Construct rename in same PR overrides DQ-R1-020 deferral without flagging

Section titled “QR1-003 — Construct rename in same PR overrides DQ-R1-020 deferral without flagging”
  • Category: Deviation
  • Severity: Medium
  • Location: analysis.md § 7.3 G-IAM-1 (L192); specification.md T-I1 step 2 (L92)
  • Observation: Both documents bundle the optional class rename (AllowCreatingNSRecordsRoleAllowCreatingDnsRecordsRole) into the same change as the generalization + byte-identity guard. decision-log.md DQ-R1-020 (L798–800) explicitly says: “The optional construct rename is name-only and can be deferred to a follow-up commit; it is not part of the byte-identical guarantee (a rename would change the CDK construct ID and thus the synthesized template’s logical IDs, breaking the unit test). If the rename is desired, it lands as a separate change after Phase 4’s role-reuse work is verified stable.” The design overrides this guidance by preserving the construct ID at the call site (new AllowCreatingDnsRecordsRole(this, "AllowCreatingNSRecordsRole", …)), which keeps the logical ID stable across the class rename. Technically sound, but the override is not flagged as a deliberate departure from DQ-R1-020.
  • Rationale: Bias-mitigation rule: when a design implies the decision-log is wrong (or partially wrong) it must say so explicitly so the next reader doesn’t trip over the contradiction. DQ-R1-020’s claim that “a rename would change the CDK construct ID” is imprecise — class renaming alone does not, provided the construct ID string is preserved. The design correctly exploits this. But the decision-log entry should be amended to reflect what was actually done, or the design should defer the rename per the original guidance.
  • References: decision-log.md § DQ-R1-020 paragraph beginning “The optional construct rename”; specification.md T-I1 step 2 (“Optional rename in the same PR (mechanical; CDK construct ID at the call site preserved)”).
  • Options:
    1. Amend decision-log.md DQ-R1-020 with a one-paragraph addendum: “Update (2026-05-12, applied at design time): The rename can land in the same change as the generalization provided the construct ID at the call site is preserved (new AllowCreatingDnsRecordsRole(this, "AllowCreatingNSRecordsRole", …)). Logical IDs are determined by the construct ID string, not the class name. Phase 4 design adopts this.” Keep the rename in the spec.
    2. Defer the rename to a follow-up PR per DQ-R1-020’s original guidance. Strip the rename from analysis.md G-IAM-1 and spec.md T-I1 step 2.
  • Recommendation: Option 1. The override is sound and the spec’s call-site mitigation is the right pattern; amending the decision-log preserves traceability and prevents the next reader from believing the rename is risky.
  • Resolution (2026-05-12): Option 1 applied. decision-log.md DQ-R1-020 (L799–803) amended with an “Update (2026-05-12, applied at design time)” paragraph explaining (a) CloudFormation logical IDs derive from the construct path (parent ID + construct ID), not the class name; (b) the Root call site preserves the construct ID as "AllowCreatingNSRecordsRole" even though the class becomes AllowCreatingDnsRecordsRole, so synthesized logical IDs are unchanged and the byte-identity guarantee holds; (c) the earlier “If the rename is desired, it lands as a separate change” note is explicitly superseded. analysis.md G-IAM-1 (L192) tightened to cite the construct-ID-preservation mechanism by name and to cross-link the decision-log update. Cascading-effects check: grep across the design package + adjacent docs found zero other locations that assume “rename deferred to follow-up” — the rename references in phases.md and elsewhere in decision-log.md are about Phase 2’s RootConfigurationRootDnsStack stack rename (different context).

QR1-006 — back-end-engineer persona suggested for TypeScript/CDK work

Section titled “QR1-006 — back-end-engineer persona suggested for TypeScript/CDK work”
  • Category: Deviation
  • Severity: Low
  • Location: plan/task-plan.md § 1 agent execution notes (L32); § 9 personas table (L220)
  • Observation: § 1 says “PRs #1, #2-5 (infrastructure side), #6: devops-engineer or back-end-engineer (Kotlin / TypeScript / CDK)”. § 9 assigns T-I1..T-I13 to devops-engineer (correctly) but the preceding § 1 wording invites back-end-engineer as an alternative. Per workspace memory feedback_ts_agent_type (recorded in MEMORY.md), back-end-engineer is reserved for Kotlin work; TypeScript / Node.js package work uses front-end-engineer, and CDK / infra work uses devops-engineer.
  • Rationale: Phase 4 introduces no Kotlin. Inviting back-end-engineer as a fallback persona for CDK / TypeScript tasks risks a misaligned agent assignment when the Team Lead spawns the implementation team.
  • References: Workspace MEMORY.md § feedback_ts_agent_type (“use front-end-engineer (not back-end-engineer) for TypeScript/Node.js package work”); project CLAUDE.md agent persona table.
  • Options:
    1. Edit L32 to drop back-end-engineer and replace with “devops-engineer for CDK + tools/ work; front-end-engineer only if any pure TypeScript packaging emerges (none currently planned)”.
    2. Leave as-is; treat the wording as historical / non-binding since § 9 (the actionable persona table) is correct.
  • Recommendation: Option 1. The actionable persona table is right; tightening the prose in § 1 prevents future re-readers from interpreting the alternative as endorsed.
  • Resolution (2026-05-12): Option 1 applied. plan/task-plan.md § 1 L32 rewritten to drop the back-end-engineer alternative and to cite workspace memory feedback_ts_agent_type explicitly (“back-end-engineer is reserved for Kotlin work and is not appropriate here”). § 9 personas table (L220) was already correct (devops-engineer for T-I1..T-I13); unchanged.

QR1-004 — Exports count: six everywhere except one stray “seven”

Section titled “QR1-004 — Exports count: six everywhere except one stray “seven””
  • Category: Inconsistency
  • Severity: Low
  • Location: analysis.md § 16 forward-references table (L570)
  • Observation: L570 says “the seven -API- exports (zone ID, zone name, two SM secret ARNs, two IAM role ARNs, Postmark token ARN)”. The enumeration sums to 7 only by double-counting the Postmark token ARN (which is itself one of the “two SM secret ARNs”). Every other location lists exactly six exports: analysis.md § 4.3 + § 12 (PartitionMailZoneId, PartitionMailZoneName) + § 5.3 G-POSTMARK-4 (EmailPostmarkAccountTokenArn) + § 6.3 G-KEY-1 (EmailEncryptionKeyArn) + § 7.3 G-IAM-3 (EmailDnsProvisioningRoleArn) + § 8.3 G-IAM-4 (EmailEncryptionKeyFallbackRoleArn); specification.md L265 (“Six export definitions”); exports.md § 1 (six rows) + L173 (“All six per-partition -API- CFN exports”).
  • Rationale: Cosmetic — the operational view is unaffected because every other doc says six. But the L570 line is a forward-reference handed to specification.md, so the slip could propagate.
  • References: exports.md § 1; specification.md L265.
  • Options:
    1. Change “seven” to “six” and rewrite the parenthetical: “(zone ID + zone name + EmailEncryptionKey ARN + Postmark token ARN + DNS role ARN + Fallback role ARN)”.
    2. Delete the parenthetical and refer to exports.md § 1 by link.
  • Recommendation: Option 1. Keep the inline breakdown — it’s useful at a glance for spec.md readers — but fix the count.
  • Resolution (2026-05-12): Option 1 applied. analysis.md § 16 (L570) rewritten: “How the operations Helm chart (or amm.sh driving Helm) reads the six -API- exports per partition (PartitionMailZoneId, PartitionMailZoneName, EmailPostmarkAccountTokenArn, EmailEncryptionKeyArn, EmailDnsProvisioningRoleArn, EmailEncryptionKeyFallbackRoleArn) …”. Cross-doc grep confirms no remaining “seven exports” / “seven -API-” references in the design package outside this review file’s audit record.
  1. Should the construct rename land in PR #1 (per spec.md) or be deferred to a follow-up (per DQ-R1-020 verbatim)? QR1-003 names the technical reasoning that makes the in-PR rename safe. The reviewer’s recommendation is to amend DQ-R1-020 in place rather than defer the rename, but the decision is the user’s; either path is workable.

  2. Is the per-partition vs per-Infrastructure DNS-records role choice settled? QR1-001 reads goal.md, decision-log.md, and specification.md as fixing per-partition. If the design ever intends one role per Alpha account (rationale: fewer roles to audit; both partitions in an account share trust-policy infrastructure), the goal-level constraint needs amending and a least-privilege rationale needs to be supplied. The reviewer’s prior is that per-partition is correct because each partition’s pod role can only assume the role scoped to its own zone.

  3. The optional fresh-agent CDK-compliance slice was not run (per § 1 working method, planned as bias-mitigation). Strong pattern compliance was observed inline during the review (three-interface pattern, validateProps + MultiError, publish() lifecycle, findOutputs("*", {Export: {Name: ...}}), CFN_IO_MARKER dual-emission witness, tryGetContext at Stack layer, Phase A / Phase B split, STS-chain + ArnLike pattern). The reviewer assessed the marginal value of a third agent attempt as low given the two prior agent thrashing failures and the explicit verification of compliance points already in the docs. If the user disagrees, this can be run as a follow-up against specification.md § 2.T-I4 + verification.md V-PART-002/017/019 specifically.

Implementation-ready with caveats. The Phase 4 design package is internally coherent on the load-bearing items: the CDK three-interface pattern, the publish() lifecycle, the findOutputs + CFN_IO_MARKER dual-emission verification shape, the STS-chained per-purpose IAM roles, the δ.1 NoEcho-CFN-parameter secret-delivery pattern, the Phase A → Phase B operator sequence, the amm.sh <infrastructure> <partition> operator surface, the register-partition-mail-signature.ts two-arg CLI with usage output, the corporate-drift preservation, and the six -API- CFN exports per partition. Cross-doc traceability holds: every REQ-NNN is referenced from at least one V-NNN; every T-NNN is bundled into a PR in specification.md § 3 and plan/task-plan.md § 3; every Phase-4 DQ-R1-NNN is referenced 12–18 times across the package.

The six findings are doc-fix-only — none requires re-architecture. QR1-001 (High) is the single gating item before implementation starts because it could lead to a wrong IAM least-privilege design if a reader takes analysis.md / requirements.md literally without cross-checking specification.md. The three Medium findings (QR1-002, QR1-003, QR1-005) are author-discipline slips — a focused one-hour doc pass addresses them. The two Low findings (QR1-004, QR1-006) are batchable cosmetic fixes.

Recommended next step: a focused doc-fix PR on jmpicnic/email-integration-phase-4 (documentation branch) that addresses QR1-001..006 in order, after which implementation can start with PR #1 (G-A workspace refactors).

Resolution (2026-05-12): All six findings addressed in a single commit on jmpicnic/email-integration-phase-4 (documentation branch). See § 4 each finding’s Resolution subsection for the edits applied. The implementation contract is now self-consistent across analysis.md, requirements.md, specification.md, verification.md, exports.md, plan/task-plan.md, and decision-log.md. PR #1 (G-A workspace refactors) is unblocked from a design-doc perspective.

Reviewer-self-disclosure: this review was performed by the author of the docs under review, with explicit bias-mitigation disciplines (§ 1). A fresh-eyes reviewer with no project history would likely surface additional findings the author missed by virtue of familiarity-blindness, particularly in areas where this review found “GOOD” without an explicit canonical-source cross-check — e.g., the discriminated-union shape on AllowCreatingNSRecordsRole.Configuration.trustPrincipal, the cross-stack import resolution from apps/Al1x/partition.ts to the partition pod role’s name pattern, and the amm.sh shell hygiene around ::add-mask:: propagation through cdk deploy --parameters. If a higher-confidence verdict is needed before PR #1 starts, a fresh-agent slice scoped narrowly to those three areas would be the highest-value follow-up.


Copyright: (c) Arda Systems 2025-2026, All rights reserved