Keep AGENTSPAN_* env vars and AgentspanError working, and validate the agent schema - #151
Closed
ambiorix2099 wants to merge 2 commits into
Closed
Keep AGENTSPAN_* env vars and AgentspanError working, and validate the agent schema#151ambiorix2099 wants to merge 2 commits into
ambiorix2099 wants to merge 2 commits into
Conversation
The Agentspan -> Conductor rename (#147) was a hard break for existing deployments. Nothing warned, and two of the three failure modes are silent: - AGENTSPAN_SERVER_URL / AUTH_KEY / AUTH_SECRET are no longer read at all. resolveOrkesConfig previously had an agent-layer tier; that tier is now gone, so an app configured this way falls through to the http://localhost:8080 default and fails against the wrong server with no indication why. - AGENTSPAN_WORKER_*, LIVENESS_*, STREAMING_*, AUTO_START_WORKERS and AGENTSPAN_LLM_MODEL silently revert to defaults. - AgentspanError was renamed with no alias, so `import { AgentspanError }` and `catch (e) { e instanceof AgentspanError }` stop compiling. That one at least fails loudly. This restores all three as deprecated fallbacks. Each legacy name maps to whatever the code reads *now* rather than reintroducing the agent-layer connection tier #147 deliberately removed -- connection/auth stays on the core CONDUCTOR_* vars, agent behaviour knobs stay on CONDUCTOR_AGENT_*. That keeps #147's design intact while making the migration non-breaking. - src/agents/legacy-env.ts readRenamedEnv(), warn-once per name - src/agents/config.ts behaviour knobs via agentEnv() - resolveOrkesConfig.ts AGENTSPAN_SERVER_URL/AUTH_* below explicit config, mapped to their CONDUCTOR_* names - src/sdk/helpers/logger.ts AGENTSPAN_LOG_LEVEL -> CONDUCTOR_LOG_LEVEL - e2e/helpers.ts its own fallback: that suite ships as a release bundle, and the SDK-level alias does not cover vars the harness reads directly - src/agents/errors.ts AgentspanError as a @deprecated alias The error alias is a direct alias, not a subclass, so `instanceof` works in both directions; the value and type are both exported so `const e: AgentspanError` still compiles. Verified against the built dist: alias and canonical are the same class object and subclasses match. AGENTSPAN_LOG_LEVEL falls back silently -- warning there would mean logging through the very logger whose level is still being resolved. The helper is duplicated between src/agents and src/sdk rather than shared, because the agent layer must not import from src/sdk outside agent-client.ts and worker.ts (AGENTS.md). Adds 14 tests. The existing suite was mechanically renamed by #147, so without these the compat guarantee would have no coverage: fallback resolution, canonical-wins precedence, empty-value handling, warn-once, and alias identity. Not addressed here, flagged only: AgentspanMetadata in the langchain and langgraph wrappers was left un-renamed by #147 while the error class was renamed. That is a naming inconsistency rather than a compat break, so it is out of scope for this change. Verified: lint 0 errors / 360 warnings (identical to main), 1580 unit tests across 80 suites (main: 1566/79), zero net new tsc errors against main, build + verify:dist clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs/agents/reference/agent-schema.json documents the wire shape AgentConfigSerializer emits and the server's agent compiler consumes, shared with the Python and Java SDKs. It shipped in #147 with nothing validating it, so it can drift from the serializer silently -- the failure mode being a documented contract that quietly stops describing reality. Adds scripts/verify-agent-schema.mjs, wired as `npm run verify:agent-schema` and run in the existing `docs` job. Follows the scripts/verify-dist.mjs pattern this repo already uses rather than transliterating python-sdk's pytest approach. It compiles the schema under its declared dialect (2020-12 here; Ajv's default export is draft-07, so the right entrypoint is selected from $schema), validates all 19 fixtures in e2e/_configs/, and asserts 8 structural invariants resolved through the root $ref so they survive restructuring: name required and identifier-patterned, agents recursive, tools referencing the shared tool def, agentConfig closed and tool open. Those last two encode the existing design rather than second-guessing it. agentConfig having additionalProperties: false is what makes this a real gate -- adding a serializer field now fails CI until the contract is updated, which is the point. Adds ajv ^8.17.1 as an explicit devDependency. It was already present transitively via eslint, but a CI gate depending on a transitive dep is how gates silently stop running. Verified: the schema compiles and all 19 fixtures pass, so the documented contract does match current serializer output. Negative-tested two ways -- a config missing `name`, and one with an undocumented extra field (caught by the closed contract) -- each fails as intended and returns to green. The existing branding grep still passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
Author
|
Duplicate work |
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
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.
Two focused follow-ups to #147. Narrowed from #148 (now closed) to only what #147 didn't cover.
1. The rename is currently a silent break —
a0235ea#147 removed the
AGENTSPAN_*names outright. Two of the three failure modes give the user nothing to go on:AGENTSPAN_SERVER_URLresolveOrkesConfig's agent-layer tier was removed entirely, not renamedhttp://localhost:8080and fails against the wrong serverAGENTSPAN_AUTH_KEY/_SECRETAGENTSPAN_WORKER_*,LIVENESS_*,STREAMING_*,AUTO_START_WORKERS,LLM_MODELAGENTSPAN_LOG_LEVELimport { AgentspanError }This restores all of them as deprecated fallbacks that warn once per name per process.
It does not re-litigate #147's design. Each legacy name maps to whatever the code reads now, rather than reintroducing the agent-layer connection tier #147 deliberately deleted — connection and auth stay on the core
CONDUCTOR_*vars, agent behaviour knobs stay onCONDUCTOR_AGENT_*. The result is that #147's shape is preserved and the migration stops being breaking.AgentspanErrorreturns as a direct alias — the same class object, not a subclass — soinstanceofworks in both directions and existingcatchblocks keep working. Value and type are both exported, soconst e: AgentspanErrorstill compiles. Verified against the builtdist/.Two deliberate asymmetries, both commented in the code:
AGENTSPAN_LOG_LEVELfalls back silently — warning there would log through the very logger whose level is still being resolved.e2e/helpers.tscarries its own fallback, because that suite ships as a release bundle for downstream repos and the SDK-level alias doesn't cover vars the harness reads directly.src/agentsandsrc/sdkrather than shared, becauseAGENTS.mdforbids the agent layer importing fromsrc/sdkoutsideagent-client.tsandworker.ts.Adds 14 tests. #147 mechanically renamed the existing suite, so without these the compat guarantee has zero coverage.
2.
agent-schema.jsonships unvalidated —284c94d#147 added
docs/agents/reference/agent-schema.json— the wire shapeAgentConfigSerializeremits, shared with the Python and Java SDKs — with nothing checking it. It can drift from the serializer silently, leaving a documented contract that quietly stops describing reality.Adds
scripts/verify-agent-schema.mjs, wired asnpm run verify:agent-schemaand run in the existingdocsjob. Follows theverify-dist.mjspattern already in this repo rather than transliterating python-sdk's pytest approach.It selects the Ajv entrypoint from
$schema(the file is 2020-12; Ajv's default export is draft-07), validates all 19e2e/_configs/fixtures, and asserts 8 invariants resolved through the root$refso they survive restructuring.Two of those invariants encode the existing design rather than second-guessing it:
agentConfigstays closed andtoolstays open.additionalProperties: falseonagentConfigis what makes this a real gate — adding a serializer field now fails CI until the contract is updated.Good news from writing it: all 19 fixtures pass, so the contract #147 documented does match current serializer output.
Adds
ajv ^8.17.1explicitly. It was already there transitively via eslint, but a CI gate resting on a transitive dep is how gates silently stop running.Verification
maintsc --noEmitmain(455 pre-existing both sides)verify:distdist/— same class object,instanceofboth waysNote
AgentspanMetadatain the langchain and langgraph wrappers was left un-renamed by #147 while the error class was renamed. That's a naming inconsistency rather than a compat break, so I left it alone — flagging it rather than widening this PR.🤖 Generated with Claude Code