feat(shared): add ctx.metadata.clear() and ctx.metadata.delete(key) fixes NV-7501#10971
Open
feat(shared): add ctx.metadata.clear() and ctx.metadata.delete(key) fixes NV-7501#10971
Conversation
…ixes NV-7501 Co-authored-by: Adam Chmara <adam.chmara1@gmail.com>
✅ Deploy preview added
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR extends the metadata signal system to support three distinct action types ( ChangesMetadata Signal Actions
Sequence DiagramsequenceDiagram
actor Agent
participant AgentContext as Agent Context
participant SignalQueue as Signal Queue
participant Validator as Signal Validator
participant Service as Metadata Service
Agent->>AgentContext: metadata.set(key, value)
AgentContext->>SignalQueue: enqueue {type: 'metadata', action: 'set', key, value}
Agent->>AgentContext: metadata.delete(key)
AgentContext->>SignalQueue: enqueue {type: 'metadata', action: 'delete', key}
Agent->>AgentContext: metadata.clear()
AgentContext->>SignalQueue: enqueue {type: 'metadata', action: 'clear'}
Agent->>AgentContext: reply() / flush()
AgentContext->>Validator: validate metadata signals
Validator->>Validator: action='set': require key & value
Validator->>Validator: action='delete': require key only
Validator->>Validator: action='clear': always valid
Validator-->>Service: valid signals
Service->>Service: apply clear (reset to {})
Service->>Service: apply delete (remove merged[key])
Service->>Service: apply set (merged[key] = value)
Service-->>Agent: updated metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 5/8 reviews remaining, refill in 22 minutes and 11 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/framework/src/resources/agent/agent.types.ts (1)
171-174: Treat the newAgentContext.metadatamethods as a semver-significant API change.Adding required members to a public exported interface can break downstream consumer mocks/implementations at compile time. Please make sure this ships under a release that allows breaking type-surface changes, or keep the addition backward-compatible.
As per coding guidelines, "packages/**/*: Treat all exported symbols in packages as public API; follow semver conventions with breaking changes requiring major bumps, new exports as minor versions, and fixes as patches."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/framework/src/resources/agent/agent.types.ts` around lines 171 - 174, You added required methods to the public AgentContext.metadata interface (set, delete, clear), which is a semver-breaking type change; either revert them or make the change backward-compatible by marking those members optional (metadata.set?, metadata.delete?, metadata.clear?) and ensure any concrete implementations (the classes/factories that construct AgentContext) provide no-op or real implementations so consumers compiling against older mocks won't break; alternatively, if you intend the breaking change, document it and ship under a major-version bump per semver.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/agents/dtos/agent-reply-payload.dto.ts`:
- Around line 55-60: The metadata_delete and metadata_clear branches currently
accept extra unrelated fields; update their validation to reject extra
properties by ensuring the signal object only contains the allowed keys: for
metadata_delete require signal.type === 'metadata_delete' AND
isValidMetadataSignalKey(signal.key) AND Object.keys(signal) is exactly
['type','key'], and for metadata_clear require signal.type === 'metadata_clear'
AND Object.keys(signal) is exactly ['type'] (do the same corresponding stricter
check in the other branch around lines 74-75). Use the existing
isValidMetadataSignalKey helper and the signal.type discriminator when
implementing the exact-key checks so the accepted shape matches
defaultMessage().
---
Nitpick comments:
In `@packages/framework/src/resources/agent/agent.types.ts`:
- Around line 171-174: You added required methods to the public
AgentContext.metadata interface (set, delete, clear), which is a semver-breaking
type change; either revert them or make the change backward-compatible by
marking those members optional (metadata.set?, metadata.delete?,
metadata.clear?) and ensure any concrete implementations (the classes/factories
that construct AgentContext) provide no-op or real implementations so consumers
compiling against older mocks won't break; alternatively, if you intend the
breaking change, document it and ship under a major-version bump per semver.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 286ff233-6f49-4c51-bbfd-b19df3319ff4
📒 Files selected for processing (8)
apps/api/src/app/agents/dtos/agent-reply-payload.dto.tsapps/api/src/app/agents/services/agent-conversation.service.tsapps/api/src/app/agents/usecases/handle-agent-reply/handle-agent-reply.usecase.tspackages/framework/src/index.tspackages/framework/src/resources/agent/agent.context.tspackages/framework/src/resources/agent/agent.test.tspackages/framework/src/resources/agent/agent.types.tspackages/framework/src/resources/agent/index.ts
Comment on lines
+55
to
+60
| if (signal.type === 'metadata_delete') { | ||
| return isValidMetadataSignalKey(signal.key); | ||
| } | ||
|
|
||
| if (signal.type === 'metadata_clear') { | ||
| return true; |
Contributor
There was a problem hiding this comment.
Reject extra fields on metadata_delete and metadata_clear.
These branches currently accept payloads with unrelated fields like value, workflowId, or payload, so malformed signals still validate and the server silently ignores the extra data. Tighten the discriminator checks so the accepted shape matches the contract in defaultMessage().
Suggested fix
if (signal.type === 'metadata_delete') {
- return isValidMetadataSignalKey(signal.key);
+ return (
+ isValidMetadataSignalKey(signal.key) &&
+ signal.value === undefined &&
+ signal.workflowId === undefined &&
+ signal.to === undefined &&
+ signal.payload === undefined
+ );
}
if (signal.type === 'metadata_clear') {
- return true;
+ return (
+ signal.key === undefined &&
+ signal.value === undefined &&
+ signal.workflowId === undefined &&
+ signal.to === undefined &&
+ signal.payload === undefined
+ );
}Also applies to: 74-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/app/agents/dtos/agent-reply-payload.dto.ts` around lines 55 -
60, The metadata_delete and metadata_clear branches currently accept extra
unrelated fields; update their validation to reject extra properties by ensuring
the signal object only contains the allowed keys: for metadata_delete require
signal.type === 'metadata_delete' AND isValidMetadataSignalKey(signal.key) AND
Object.keys(signal) is exactly ['type','key'], and for metadata_clear require
signal.type === 'metadata_clear' AND Object.keys(signal) is exactly ['type'] (do
the same corresponding stricter check in the other branch around lines 74-75).
Use the existing isValidMetadataSignalKey helper and the signal.type
discriminator when implementing the exact-key checks so the accepted shape
matches defaultMessage().
…types
Replace MetadataSetSignal, MetadataDeleteSignal, MetadataClearSignal with
a single MetadataSignal union discriminated by an action field:
{ type: 'metadata', action?: 'set', key, value }
{ type: 'metadata', action: 'delete', key }
{ type: 'metadata', action: 'clear' }
This keeps Signal = MetadataSignal | TriggerSignal (no union expansion),
simplifies server-side filters (s.type === 'metadata' still catches all),
and is backward compatible (action defaults to 'set' when omitted).
Co-authored-by: Adam Chmara <adam.chmara1@gmail.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.
What changed
Added
ctx.metadata.clear()andctx.metadata.delete(key)methods to the agent context, enabling developers to reset or selectively remove conversation metadata from handlers.Problem
Previously, there was no way to clear or delete individual metadata keys. The only workaround was manually setting every known key to
null:Solution
Two new methods on
ctx.metadata:Architecture: action discriminant on a single signal type
Rather than introducing separate top-level signal types (
metadata_delete,metadata_clear), the new operations use anactiondiscriminant within the existingmetadatasignal:Why this is better than separate signal types:
SignalstaysMetadataSignal | TriggerSignal— no union expansions.type === 'metadata') work unchangedSIGNAL_TYPESstays['metadata', 'trigger']— no new constants neededactiondefaults to'set'when omitted (existing SDKs unaffected)merge,increment) are newactionvalues, not new top-level typesChanges
Framework SDK (
packages/framework)MetadataSignalis now a discriminated union onaction(set | delete | clear)AgentContext.metadatainterface extended withdelete(key)andclear()AgentContextImplpushes the appropriateactionfor each methodAPI server (
apps/api)SignalDtogains an optionalactionfield; validator branches on it withintype === 'metadata'updateMetadataprocessesclear(reset to{}),delete(remove key), andsetin ordervalidateMetadataSignalKeysskips value check fordeleteactionTests
action: 'set'fieldLinear Issue: NV-7501
What changed
This PR adds
ctx.metadata.delete(key)andctx.metadata.clear()methods to the agent context metadata API. Previously, clearing metadata required manually setting known keys to null. Now developers can remove individual keys or wipe all metadata with dedicated methods. These new operations emit metadata signals that integrate seamlessly with the existing batching and flushing pipeline.Affected areas
framework: Extended
AgentContext.metadatainterface withdelete(key)andclear()methods. UpdatedMetadataSignaltype to a discriminated union supportingaction: 'set' | 'delete' | 'clear', with'set'as the default when omitted for backward compatibility.api: Updated
SignalDtovalidation to accept metadata signals with action variants. ModifiedupdateMetadatato processclear(resets metadata to{}),delete(removes single key), andsetoperations in order.Key technical decisions
MetadataSignalunion (action: 'set' | 'delete' | 'clear') instead of separate signal types, reducing the overallSignalunion complexity and simplifying server-side filtering logic.reply()/flush()pipeline rather than requiring a separate API path.actionfield defaults to'set'when omitted, maintaining backward compatibility with existing code.Testing
Added 4 new Vitest cases covering:
metadata.delete()batched with reply,metadata.clear()batched with reply, mixedclear+set+deleteoperations preserving order, andmetadata.clear()flushed after handler completion. All 316 framework tests pass.