refactor(code-runtime): address seam review — drop worker aliases, tighten dunder
- Worker consumes PORTABLE_RESERVED_WORDS / RESERVED_ERROR_MEMBERS by their seam names directly, dropping the local re-alias (symmetry with the other two imported constants). - Split the reserved-vs-duplicate diagnostics: a backend-owned global now reports "reserved binding global", not the misleading "duplicate". - DUNDER_MEMBER uses `__.+__` so a bare `__` (empty middle, not a real CPython dunder) is not matched; add coverage. - Worker misuse tests add `a$b` (second-char `$`) and `lambda` (Python keyword) so the identifier narrowing and reserved-word adoption are each pinned directly, not only transitively. - Clarify the seam JSDoc (dunder-vs-explicit-set wording, Python backend is a later stack PR) and record in the Agent Note the obligation to widen RESERVED_BINDING_GLOBALS when the bootstrap seeds more globals.
This commit is contained in:
Chinesezjc
@@ -65,14 +65,6 @@ const ELU_POLL_INTERVAL_MS = 25
|
||||
/** Smallest cap that can represent the counted payloads: an empty logs array plus an empty JSON failure message. */
|
||||
const MIN_OUTPUT_BYTES = 4
|
||||
|
||||
/**
|
||||
* The seam's cross-language reserved-word union: the portable-identifier
|
||||
* contract promises a namespace list valid here is valid on every backend, so
|
||||
* a Python keyword like `lambda` is refused even though it is a legal JS
|
||||
* parameter name.
|
||||
*/
|
||||
const RESERVED_WORDS = PORTABLE_RESERVED_WORDS
|
||||
|
||||
/**
|
||||
* The seam's language-portable identifier subset (see
|
||||
* `CodeBindingNamespace.global`): no `$`, which is JS-only spelling — the same
|
||||
@@ -80,13 +72,6 @@ const RESERVED_WORDS = PORTABLE_RESERVED_WORDS
|
||||
*/
|
||||
const IDENTIFIER = /^[A-Za-z_][A-Za-z0-9_]*$/
|
||||
|
||||
/**
|
||||
* The seam's shared error-member exclusions (plus the dunder rule below):
|
||||
* enforced identically here and in the Python backend so an errorClass valid
|
||||
* on one backend is valid on all.
|
||||
*/
|
||||
const RESERVED_ERROR_PROPERTIES = RESERVED_ERROR_MEMBERS
|
||||
|
||||
/**
|
||||
* The shell a program is wrapped in for the type-strip, matching the
|
||||
* grammatical context it will execute in (an async function body, where
|
||||
@@ -335,14 +320,17 @@ export class WorkerCodeRuntime extends CodeRuntime {
|
||||
private validateBindings(request: CodeRunRequest): Map<string, CodeBindingNamespace> {
|
||||
const bindings = new Map<string, CodeBindingNamespace>()
|
||||
for (const namespace of request.bindings) {
|
||||
if (!IDENTIFIER.test(namespace.global) || RESERVED_WORDS.has(namespace.global)) {
|
||||
if (!IDENTIFIER.test(namespace.global) || PORTABLE_RESERVED_WORDS.has(namespace.global)) {
|
||||
throw new Error(`dsh-code-runtime-worker: binding global ${JSON.stringify(namespace.global)} is not a usable identifier`)
|
||||
}
|
||||
// RESERVED_BINDING_GLOBALS is the seam's shared backend-owned set:
|
||||
// `console` is THIS backend's log-capture slot; the dunder entries are
|
||||
// the Python bootstrap's — refused here too so the namespace list stays
|
||||
// the Python backend's — refused here too so the namespace list stays
|
||||
// portable across backends.
|
||||
if (RESERVED_BINDING_GLOBALS.has(namespace.global) || bindings.has(namespace.global)) {
|
||||
if (RESERVED_BINDING_GLOBALS.has(namespace.global)) {
|
||||
throw new Error(`dsh-code-runtime-worker: reserved binding global ${JSON.stringify(namespace.global)}`)
|
||||
}
|
||||
if (bindings.has(namespace.global)) {
|
||||
throw new Error(`dsh-code-runtime-worker: duplicate binding global ${JSON.stringify(namespace.global)}`)
|
||||
}
|
||||
bindings.set(namespace.global, namespace)
|
||||
@@ -352,14 +340,17 @@ export class WorkerCodeRuntime extends CodeRuntime {
|
||||
for (const namespace of request.bindings) {
|
||||
const descriptor = namespace.errorClass
|
||||
if (!descriptor) continue
|
||||
if (!IDENTIFIER.test(descriptor.name) || RESERVED_WORDS.has(descriptor.name)) {
|
||||
if (!IDENTIFIER.test(descriptor.name) || PORTABLE_RESERVED_WORDS.has(descriptor.name)) {
|
||||
throw new Error(`dsh-code-runtime-worker: binding error class ${JSON.stringify(descriptor.name)} is not a usable identifier`)
|
||||
}
|
||||
if (RESERVED_BINDING_GLOBALS.has(descriptor.name) || bindings.has(descriptor.name) || errorClassNames.has(descriptor.name)) {
|
||||
if (RESERVED_BINDING_GLOBALS.has(descriptor.name)) {
|
||||
throw new Error(`dsh-code-runtime-worker: reserved binding global ${JSON.stringify(descriptor.name)}`)
|
||||
}
|
||||
if (bindings.has(descriptor.name) || errorClassNames.has(descriptor.name)) {
|
||||
throw new Error(`dsh-code-runtime-worker: duplicate injected global ${JSON.stringify(descriptor.name)}`)
|
||||
}
|
||||
const member = descriptor.memberNameProperty
|
||||
if (member.length === 0 || RESERVED_ERROR_PROPERTIES.has(member) || DUNDER_MEMBER.test(member)) {
|
||||
if (member.length === 0 || RESERVED_ERROR_MEMBERS.has(member) || DUNDER_MEMBER.test(member)) {
|
||||
throw new Error(`dsh-code-runtime-worker: binding error member property ${JSON.stringify(descriptor.memberNameProperty)} is not usable`)
|
||||
}
|
||||
errorClassNames.add(descriptor.name)
|
||||
|
||||
@@ -790,7 +790,14 @@ describe('WorkerCodeRuntime — seam misuse and lifecycle', () => {
|
||||
// `$tools` is legal JS but outside the seam's language-portable subset:
|
||||
// the same namespace list must work against every backend's language.
|
||||
['$tools', /not a usable identifier/],
|
||||
['console', /duplicate binding global/],
|
||||
// `a$b` pins the second character class too: the old identifier regex
|
||||
// `[A-Za-z0-9_$]*` would have accepted a `$` after the first character.
|
||||
['a$b', /not a usable identifier/],
|
||||
// `lambda` is a Python keyword, refused here directly (not just
|
||||
// transitively) so the worker's adoption of PORTABLE_RESERVED_WORDS is
|
||||
// its own regression, symmetric with the `$tools` case.
|
||||
['lambda', /not a usable identifier/],
|
||||
['console', /reserved binding global/],
|
||||
]
|
||||
for (const [global, message] of cases) {
|
||||
await expect(runtime.run({ program: 'return 1', bindings: [{ global, functions: {} }] })).rejects.toThrow(message)
|
||||
@@ -817,7 +824,7 @@ describe('WorkerCodeRuntime — seam misuse and lifecycle', () => {
|
||||
|
||||
await expect(run([namespace('tools', 'not valid!')])).rejects.toThrow(/error class.*not a usable identifier/)
|
||||
await expect(run([namespace('tools', 'await')])).rejects.toThrow(/error class.*not a usable identifier/)
|
||||
await expect(run([namespace('tools', 'console')])).rejects.toThrow(/duplicate injected global/)
|
||||
await expect(run([namespace('tools', 'console')])).rejects.toThrow(/reserved binding global/)
|
||||
await expect(run([namespace('tools', 'tools')])).rejects.toThrow(/duplicate injected global/)
|
||||
await expect(run([
|
||||
namespace('tools', 'CallError'),
|
||||
@@ -829,10 +836,10 @@ describe('WorkerCodeRuntime — seam misuse and lifecycle', () => {
|
||||
// dunders too, so the same errorClass is valid (or not) on every backend.
|
||||
await expect(run([namespace('tools', 'CallError', 'args')])).rejects.toThrow(/member property.*not usable/)
|
||||
await expect(run([namespace('tools', 'CallError', '__dict__')])).rejects.toThrow(/member property.*not usable/)
|
||||
// The Python bootstrap's owned globals are refused here too (shared
|
||||
// The Python backend's owned globals are refused here too (shared
|
||||
// RESERVED_BINDING_GLOBALS), keeping namespace lists backend-portable.
|
||||
await expect(runtime.run({ program: 'return 1', bindings: [{ global: '__dsh_main__', functions: {} }] }))
|
||||
.rejects.toThrow(/duplicate binding global/)
|
||||
.rejects.toThrow(/reserved binding global/)
|
||||
})
|
||||
|
||||
it('rejects config values that are not positive numbers', async () => {
|
||||
|
||||
@@ -20,19 +20,23 @@ export type {
|
||||
/**
|
||||
* Binding globals EVERY backend refuses because SOME backend owns the slot in
|
||||
* the program's namespace: `console` (the worker's log capture), and
|
||||
* `__dsh_main__`/`__builtins__`/`__name__` (the Python bootstrap's wrapper
|
||||
* and seeded module globals), and `__debug__`. One shared set — rather than each backend
|
||||
* refusing only its own slots — keeps the portability promise real: a
|
||||
* namespace list valid on one backend is valid on all, so a caller cannot
|
||||
* pick a name that works on the worker and collides on Python (or vice
|
||||
* versa). Dunder-form names are additionally covered by the identifier rule
|
||||
* on `CodeBindingNamespace.global` only when they fail it; `__name__` et al.
|
||||
* ARE valid identifiers, hence this explicit set. `__debug__` is listed for a
|
||||
* different reason than a collision: CPython compiles a bare `__debug__`
|
||||
* reference to the constant `True` and rejects any assignment to the name at
|
||||
* COMPILE time, so an injected global under that name is unreachable from the
|
||||
* program — accepted by validation, unusable on the Python backend, which is
|
||||
* exactly the split the shared set exists to prevent.
|
||||
* `__dsh_main__`/`__builtins__`/`__name__` (the Python backend's bootstrap
|
||||
* wrapper and seeded module globals — that backend is a later PR in this
|
||||
* stack, see the [portable-identifier Agent
|
||||
* Note](../../../../.agents/notes/implemented/architecture/2026-07-31-code-runtime-portable-identifier-seam.md)),
|
||||
* and `__debug__`. One shared set — rather than each backend refusing only its
|
||||
* own slots — keeps the portability promise real: a namespace list valid on
|
||||
* one backend is valid on all, so a caller cannot pick a name that works on
|
||||
* the worker and collides on Python (or vice versa). `__name__` et al. ARE
|
||||
* valid portable identifiers, so the identifier rule on
|
||||
* `CodeBindingNamespace.global` never rejects them — hence this explicit set.
|
||||
* (Error members differ: {@link DUNDER_MEMBER} refuses every dunder form
|
||||
* wholesale; binding globals refuse only the names listed here.) `__debug__`
|
||||
* is listed for a different reason than a collision: CPython compiles a bare
|
||||
* `__debug__` reference to the constant `True` and rejects any assignment to
|
||||
* the name at COMPILE time, so an injected global under that name is
|
||||
* unreachable from the program — accepted by validation, unusable on the
|
||||
* Python backend, which is exactly the split the shared set exists to prevent.
|
||||
*/
|
||||
export const RESERVED_BINDING_GLOBALS: ReadonlySet<string> = new Set([
|
||||
'console',
|
||||
@@ -54,8 +58,11 @@ export const RESERVED_ERROR_MEMBERS: ReadonlySet<string> = new Set([
|
||||
'args', 'with_traceback', 'add_note',
|
||||
])
|
||||
|
||||
/** Dunder form (`__*__`): object-protocol slots in Python, refused as {@link RESERVED_ERROR_MEMBERS | error members} on every backend. */
|
||||
export const DUNDER_MEMBER = /^__.*__$/
|
||||
/**
|
||||
* Dunder form (`__x__`, non-empty middle): object-protocol slots in Python,
|
||||
* refused as {@link RESERVED_ERROR_MEMBERS | error members} on every backend.
|
||||
*/
|
||||
export const DUNDER_MEMBER = /^__.+__$/
|
||||
|
||||
/**
|
||||
* Reserved words of EVERY shipped backend language (ECMAScript ∪ Python),
|
||||
|
||||
@@ -35,6 +35,9 @@ describe('seam-owned portable identifier exclusions', () => {
|
||||
expect(DUNDER_MEMBER.test('_private')).toBe(false)
|
||||
expect(DUNDER_MEMBER.test('name')).toBe(false)
|
||||
expect(DUNDER_MEMBER.test('__mid')).toBe(false)
|
||||
// `__` has an empty middle — not a real CPython dunder, so not matched.
|
||||
expect(DUNDER_MEMBER.test('__')).toBe(false)
|
||||
expect(DUNDER_MEMBER.test('____')).toBe(true)
|
||||
})
|
||||
|
||||
it('PORTABLE_RESERVED_WORDS is the union of ECMAScript and Python reserved words', () => {
|
||||
|
||||
Reference in New Issue
Block a user