docs(spec): clarify RFCs based on pilot-implementation findings (iter-11)

tercel · claude · tercel · commit 81df33618f39 · 2026-05-06T00:57:30.000+08:00
Round 1+2 sub-agent SDK pilot implementations of the Stage-2/Stage-3 RFCs surfaced 5 ambiguities and 1 factual error in the RFC text. All clarifications landed as same-day RFC edits to keep the spec consistent with the pilot PRs. rfc-preview-method.md changes: - Pre-conditions section: replace incorrect "..Default::default() (FRU)" migration text with the correct "Default + field assignment" pattern. Per Rust E0639, #[non_exhaustive] blocks struct-literal construction from external crates entirely, including FRU. Apcore-rust v0.21.0 CHANGELOG (PR #25) is now the canonical migration reference. - Add cross-SDK forward-compat-semantics table showing why this is a Rust-only concern (Python dataclass + TS interface get forward-compat for free; Rust has the only declaration mechanism + the only cost). - Change.x-* extension fields: add cross-SDK schema-encoding note. RFC does NOT prescribe runtime validation; lists idiomatic encodings (pydantic / serde-flatten / Type.Unsafe) and conformance-fixture requirements covering required-only and x-foo round-trip cases. rfc-ephemeral-modules.md changes: - Add "Audit-event single-emit rule" section: for ephemeral.* modules, exactly one event MUST be emitted with full contextual payload (module_id, caller_id, identity?, namespace_class). Avoids dual-emit with pre-existing empty-payload registry-event bridge across all 3 SDKs. - Add "register_internal() interaction" rule: register_internal MUST reject ephemeral.* IDs and direct callers to Registry.register(). Preserves audit-trail provenance distinction and ACL enforcement. - Add "Transitional fixture handling" section: don't update annotations_extra_round_trip.json until all 3 SDKs ship discoverable; pilot-tolerant test pattern from apcore-python#26 is the bridge. decision-log.md: iter-11 addendum recording all 5 clarifications with their cross-references to the pilot PRs that surfaced them. No SDK behavior change. RFCs remain Draft / RFC. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: tercel <tercel.yi@gmail.com>
diff --git a/docs/spec/2026-05-decision-log.md b/docs/spec/2026-05-decision-log.md
@@ -1075,6 +1075,62 @@ No SDK behavior changes, no normative `MUST` / `MUST NOT` additions, no schema f
 
 ---
 
+### Resolution status — iter-11 addendum (RFC clarifications post pilot-implementation, 2026-05-05)
+
+A round of parallel sub-agent SDK pilot implementations against the two Stage-2/Stage-3 RFCs (`rfc-preview-method.md`, `rfc-ephemeral-modules.md`) surfaced concrete ambiguities and one factual error in the RFC text. Resolutions landed as same-day RFC edits on `main`. Pilot PRs:
+
+- `apcore-rust#25` — `#[non_exhaustive]` hygiene for spec-derived public structs (Stage 2 prereq)
+- `apcore-python#26` — `ephemeral.*` namespace + `discoverable` annotation pilot (Stage 3)
+- `apcore-typescript#29` — `Module.preview()` implementation (Stage 2)
+- `apcore-typescript#28` → PR `#30` — `multiClassEnabled` arg drop (D-06 follow-through)
+
+#### Clarification 1 — `rfc-preview-method.md` Pre-conditions: `#[non_exhaustive]` migration pattern
+
+**Surfaced by**: `apcore-rust#25` agent during implementation.
+
+**Issue**: The RFC's earlier text claimed downstream consumers should use `..Default::default()` (functional record update / FRU) syntax to construct extended structs. **This is factually wrong.** Per Rust's [E0639], `#[non_exhaustive]` blocks struct-literal construction from outside the defining crate **entirely**, including FRU. The `..Default::default()` pattern works only **within** the defining crate.
+
+**Resolution**: RFC Pre-conditions section rewritten with the correct migration pattern (`Default::default()` + field assignment) and a cross-SDK forward-compat-semantics table showing why this is a Rust-only concern. The `apcore-rust` v0.21.0 CHANGELOG (PR #25) is the canonical reference for the migration code example.
+
+#### Clarification 2 — `rfc-ephemeral-modules.md`: single-emit audit rule
+
+**Surfaced by**: `apcore-python#26` agent.
+
+**Issue**: All three SDKs have a pre-existing registry-event bridge (`_bridge_registry_events` in Python; equivalents in TypeScript and Rust) that emits `apcore.registry.module_registered` / `module_unregistered` with empty payload. Naively adding a second contextual emit for `ephemeral.*` produced dual-emit of the same `event_type` for the same registration.
+
+**Resolution**: RFC §"Audit-event single-emit rule" added. For `ephemeral.*` registrations, exactly **one** event MUST be emitted, carrying the contextual payload (`module_id`, `caller_id` defaulting to `"@external"`, redacted `identity` snapshot, `namespace_class: ephemeral`). For non-`ephemeral.*` registrations, the existing bridge's empty-payload behavior is preserved. Implementation detail (short-circuit bridge vs. extend bridge) is per-SDK.
+
+#### Clarification 3 — `rfc-ephemeral-modules.md`: `register_internal()` rejection
+
+**Surfaced by**: `apcore-python#26` agent.
+
+**Issue**: Python's `register_internal()` historically reserved for `system.*` modules. The RFC didn't explicitly state whether `ephemeral.*` IDs could be registered via this backdoor.
+
+**Resolution**: RFC §"`register_internal()` interaction" added. `register_internal()` (or equivalent) **MUST reject** `ephemeral.*` IDs and direct callers to `Registry.register()`. Rationale: namespace → registration-mechanism is a 1:1 mapping; mixing blurs the audit-trail distinction between framework-emitted (`system.*`) and caller-emitted (`ephemeral.*`) events.
+
+#### Clarification 4 — `rfc-preview-method.md`: `Change.x-*` schema encoding
+
+**Surfaced by**: `apcore-typescript#29` agent during TypeBox schema definition.
+
+**Issue**: TypeBox 0.34 doesn't natively express the TypeScript template-literal index signature `[key: \`x-${string}\`]: unknown` for `Change`. Naive `Type.Object()` rejects `x-*` keys; `Type.Intersect([Object, Record])` loses `additionalProperties: false` precision.
+
+**Resolution**: RFC §"`Change.x-*` extension fields — cross-SDK schema-encoding note" added. The spec does NOT prescribe a runtime-validation mechanism; per-SDK guidance:
+- Python: `pydantic` `extra='allow'` + `^x-` validator
+- Rust: `#[serde(flatten)] extra: HashMap` + custom validator
+- TypeScript: `Type.Unsafe<Change>(...)` injecting raw JSON Schema `patternProperties: { "^x-": {} }` (escape-hatch; preserves TS type)
+
+apcore-typescript PR #29 author noted this in JSDoc; follow-up to upgrade the TypeBox schema to `Type.Unsafe` is straightforward.
+
+#### Clarification 5 — `rfc-ephemeral-modules.md`: transitional conformance-fixture handling
+
+**Surfaced by**: `apcore-python#26` agent (made conformance test pilot-tolerant for missing `discoverable`).
+
+**Issue**: Updating `conformance/fixtures/annotations_extra_round_trip.json` to require the new `discoverable` field would actively break the conformance tests of SDKs that haven't shipped support yet (TypeScript and Rust during the rollout window).
+
+**Resolution**: RFC §"Transitional fixture handling during multi-SDK rollout" added. SDKs implementing `discoverable` MAY make their conformance test runner pilot-tolerant (strip the field from comparison when `expected_serialized` lacks it). Once **all three SDKs** ship support, a synchronized follow-up PR updates the fixture and removes pilot-tolerant code paths.
+
+---
+
 ### Resolution status — iter-10 addendum (D-06 doc-side, 2026-05-05)
 
 - **D-06 (doc-side)** — resolved. `docs/features/multi-module-discovery.md` "Enabling Multi-Class Mode" section rewritten to drop the **Global opt-in (configuration)** paragraph that referenced the unimplemented `extensions.multi_class_discovery` config key. Per-class markers (`@multi_class` / `@multiClass()` / `#[multi_class]`) are now documented as the only opt-in path, with an inline backward-reference note pointing readers to this decision-log entry.
diff --git a/docs/spec/rfc-ephemeral-modules.md b/docs/spec/rfc-ephemeral-modules.md
@@ -57,7 +57,7 @@ Modules in the `ephemeral.*` namespace:
 - **SHOULD** declare `requires_approval: true` to prevent agent-synthesized tools from running unattended.
 - **SHOULD** declare `discoverable: false` (a new annotation; see below).
 - **MUST** be subject to ACL `default_effect: deny`. ACL `target` patterns supporting `ephemeral.*` wildcards already work (per existing first-match-wins evaluation).
-- **MUST** emit audit events through the framework `EventEmitter`, mirroring `system.control.*` write modules' contextual-audit shape (D-35).
+- **MUST** emit audit events through the framework `EventEmitter`, mirroring `system.control.*` write modules' contextual-audit shape (D-35). See "Single-emit rule" below.
 - **SHOULD** declare a TTL (open question: convention key — see below).
 
 ## Proposed new `discoverable` annotation
@@ -76,6 +76,41 @@ Semantics:
 
 This is independently useful: filesystem-rooted modules can also opt out of enumeration (e.g., for internal-only utilities), and existing `internal: true` patterns in some SDK ports converge on the same idea.
 
+## Audit-event single-emit rule — **clarification (post-pilot)**
+
+A four-round audit during the apcore-python pilot surfaced a dual-emit risk: all three SDKs already have a registry-event bridge (`apcore-python` `_bridge_registry_events`, `apcore-typescript` `sys-modules/registration.ts`, `apcore-rust` `RegistryEvents` callback) that fires `apcore.registry.module_registered` / `apcore.registry.module_unregistered` with an **empty payload** for every registration. Naïvely adding a second contextual emit for `ephemeral.*` registrations produces two events with the same `event_type` for the same registration — bad audit hygiene.
+
+**Rule**: For any single registration / unregistration of an `ephemeral.*` module, exactly **one** event MUST be emitted, carrying the full contextual payload:
+
+```yaml
+event_type: apcore.registry.module_registered  # or .module_unregistered
+payload:
+  module_id: ephemeral.<name>
+  caller_id: <string>          # defaults to "@external" when context.caller_id is None/null/""
+  identity: <object | null>    # redacted snapshot per D-35; null when no identity is set
+  namespace_class: ephemeral
+```
+
+For non-`ephemeral.*` registrations, the existing bridge's empty-payload behavior is preserved (backward compatibility for current subscribers).
+
+**Implementation choices** (per-SDK, not normative):
+- Short-circuit the bridge for `ephemeral.*` IDs and emit only the rich version, OR
+- Extend the bridge to be context-aware for all registrations (richer for everyone, opt-in by the SDK)
+
+Subscribers SHOULD treat additional payload fields as forward-compatible (existing empty-payload subscribers continue working without modification).
+
+## `register_internal()` interaction — **clarification (post-pilot)**
+
+`apcore-python` exposes `register_internal()` as a programmatic registration path historically reserved for `system.*` modules. The pilot surfaced an open question: should `register_internal()` accept `ephemeral.*` IDs?
+
+**Rule**: `register_internal()` (or its equivalent in any SDK) **MUST reject** `ephemeral.*` IDs with a clear error pointing the caller to use `Registry.register()`. Rationale:
+
+1. **Audit-trail provenance**: `system.*` events carry framework-emitted provenance; `ephemeral.*` events carry caller-emitted (Agent / user) provenance. Mixing the two backdoors blurs forensics.
+2. **ACL enforcement**: standard `register()` runs the full ACL + audit pipeline; `register_internal()` typically bypasses ACL because system modules are framework-owned. `ephemeral.*` modules are **not** framework-owned and MUST go through ACL.
+3. **Principle**: namespace prefix → registration mechanism is a 1:1 mapping. `system.*` only via `register_internal()`. `ephemeral.*` only via `register()`. No overlap.
+
+For SDKs that don't have a `register_internal()` distinction (`apcore-typescript`, `apcore-rust`), this rule is automatically satisfied.
+
 ## Lifecycle / GC contract — **open**
 
 Two designs are on the table:
@@ -107,6 +142,17 @@ The spec's responsibility is bounded at registration → ACL → audit → lifec
 
 ## Conformance plan (for post-acceptance)
 
+### Transitional fixture handling during multi-SDK rollout
+
+The `discoverable` annotation field is a normative addition to `ModuleAnnotations` once this RFC is accepted. During the rollout window where some SDKs have shipped `discoverable` and others have not, the canonical conformance fixture `conformance/fixtures/annotations_extra_round_trip.json` MUST NOT be updated to require the field — doing so would actively break the conformance tests of SDKs that have not yet shipped support. Instead:
+
+- SDKs that ship `discoverable` MAY make their conformance test runner pilot-tolerant: when comparing an `expected_serialized` block that lacks `discoverable`, strip the field from the actual serialized output before equality comparison.
+- Once **all three SDKs** have shipped `discoverable`, a follow-up apcore PR updates `annotations_extra_round_trip.json`: add `discoverable: true` (the default) to each test case's `input` and `expected_serialized`, and add a new test case `discoverable_false_round_trip` covering the explicit-false path. After that PR lands, the pilot-tolerant code paths in each SDK can be removed.
+
+`apcore-python` PR #26 (the pilot) implements the pilot-tolerant pattern. `apcore-typescript` and `apcore-rust` follow-up PRs SHOULD do the same until the synchronized fixture update.
+
+### Per-feature fixture (post-acceptance)
+
 `conformance/fixtures/ephemeral_modules.json` (not created in this RFC stage):
 
 1. Register `ephemeral.test_v1` programmatically; assert `Registry.list()` does not include it; `Registry.invoke("ephemeral.test_v1", ...)` succeeds.
diff --git a/docs/spec/rfc-preview-method.md b/docs/spec/rfc-preview-method.md
@@ -95,6 +95,18 @@ Change:
 
 The required `summary` field is the floor: even a module that wraps an opaque external API can produce `{action: "send", target: "smtp:user@example.com", summary: "Send order confirmation email to user@example.com"}`.
 
+### `Change.x-*` extension fields — cross-SDK schema-encoding note
+
+The `Change` object MAY contain any number of `^x-`-prefixed keys with arbitrary values, mirroring the §4.6 metadata-extension convention. **This RFC does not prescribe a runtime-validation mechanism**; each SDK uses an idiomatic encoding:
+
+| SDK | Idiomatic encoding | Note |
+|-----|-------------------|------|
+| `apcore-python` | `pydantic.BaseModel` with `model_config = ConfigDict(extra='allow')`, plus a model-validator that asserts unknown keys match `^x-` | Native; round-trips through `model_dump()` |
+| `apcore-rust` | `#[serde(flatten)] extra: HashMap<String, Value>` + custom validator at construction time | Native; flatten preserves wire format |
+| `apcore-typescript` | TypeBox 0.34 has no native template-literal index keys for `Type.Object`; use `Type.Unsafe<Change>(...)` to inject raw JSON Schema `patternProperties: { "^x-": {} }` while preserving the TypeScript type. `Type.Intersect([Object, Record])` is **not** equivalent because it loses `additionalProperties: false`. | Escape-hatch; TypeBox idiom |
+
+Conformance fixtures MUST cover at least: (a) a `Change` with required fields only and no `x-*` keys; (b) a `Change` with one `x-foo: <value>` key that round-trips identically. This guards against an SDK silently dropping `x-*` keys during serialization.
+
 ## Proposed `PreflightResult` extension
 
 A new optional field on the existing `PreflightResult` (PROTOCOL_SPEC.md §12.8.4 type table):
@@ -201,9 +213,35 @@ PreflightResult:
 1. Open a tracking issue against `apcore-rust` titled "Mark spec-derived public structs `#[non_exhaustive]`".
 2. List `PreflightResult`, `PreflightCheckResult`, and any other spec-derived public struct that may be extended.
 3. Land the attribute change as a single commit in `apcore-rust` minor bump (v0.21.0 of the SDK is acceptable).
-4. Document in `apcore-rust` migration notes: downstream consumers must use struct-update syntax (`..Default::default()`) or builder methods.
 
-This concern does not arise in `apcore-python` (dataclasses tolerate added fields with defaults) or `apcore-typescript` (interfaces with optional `?` properties are forward-compatible).
+### Migration pattern for downstream Rust consumers — **important: not what you'd guess**
+
+A common misconception (this RFC's earlier text included) is that `..Default::default()` (functional record update / FRU) bypasses `#[non_exhaustive]`. **It does not.** Per Rust's `E0639`, `#[non_exhaustive]` blocks struct-literal construction from outside the defining crate **entirely**, including FRU syntax. Within the crate that defines the struct, FRU still works.
+
+**Correct downstream migration pattern**:
+
+```rust
+// ❌ NOT permitted from external crates (E0639):
+let r = PreflightResult { valid: true, checks: vec![], ..Default::default() };
+
+// ✅ Use Default + field assignment:
+let mut r = PreflightResult::default();
+r.valid = true;
+r.checks = vec![];
+// (or a future builder API the SDK may add)
+```
+
+The `apcore-rust` v0.21.0 CHANGELOG entry (commit landing alongside the `#[non_exhaustive]` attribute) is the canonical reference for the migration pattern. SDKs MAY ship a builder API in a follow-up minor; this RFC does not mandate one.
+
+### Cross-SDK forward-compat semantics
+
+| SDK | Adding a field is breaking? | Mechanism |
+|-----|----------------------------|-----------|
+| `apcore-python` | ❌ No | `dataclass` field with default; existing keyword-arg call sites unaffected |
+| `apcore-typescript` | ❌ No | `interface { foo?: T }` is forward-compatible by structural-typing |
+| `apcore-rust` | ⚠️ Yes — without `#[non_exhaustive]` declared upfront | Rust's only forward-compat declaration mechanism; cost is downstream construction-syntax tax (E0639) |
+
+This asymmetry is **why** the `#[non_exhaustive]` work is staged as a Rust-specific pre-condition; the other two SDKs need no preparation step.
 
 ## Conformance plan
 
