feat(config): add optional metadata dict to workflow definition#107
feat(config): add optional metadata dict to workflow definition#107PolyphonyRequiem wants to merge 8 commits intomicrosoft:mainfrom
Conversation
Add a metadata field to WorkflowDef that allows workflow authors to
attach arbitrary key-value pairs for external tooling. The metadata
is included verbatim in the workflow_started event, enabling
downstream consumers (dashboards, trackers, enrichers) to adapt
behavior without parsing the YAML source.
Example usage in workflow YAML:
workflow:
name: twig-sdlc
metadata:
tracker: ado
project_url: https://dev.azure.com/org/Project
work_item_id_agent: intake
work_item_id_field: epic_id
The field defaults to an empty dict, so existing workflows are
unaffected.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add --metadata / -m flag to 'conductor run' that accepts key=value
pairs, merged on top of YAML-declared metadata. This enables callers
to inject dynamic values at invocation time:
conductor run twig-sdlc.yaml --metadata work_item_id=1814
CLI metadata is:
- Parsed separately from --input (different binding path)
- Merged on top of YAML metadata (CLI wins on conflicts)
- Forwarded through --web-bg background process spawning
- Included in the workflow_started event alongside YAML metadata
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
7 new tests verifying: - Schema: metadata defaults to empty dict, accepts arbitrary keys, independent from input/context fields - Loader: metadata round-trips through YAML, omission gives empty dict, nested values preserved, metadata and input are separate namespaces All 140 config tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #107 +/- ##
=======================================
Coverage ? 84.67%
=======================================
Files ? 53
Lines ? 7232
Branches ? 0
=======================================
Hits ? 6124
Misses ? 1108
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Propagate the event log's random hex suffix as a run_id across all systems: - EventLogSubscriber: expose run_id property (was already generated) - WorkflowEngine: accept run_id + log_file params, include in workflow_started event - PID files: include run_id + log_file fields - Web dashboard: add /api/info endpoint returning run_id, log_file, workflow_name, started_at, metadata This enables the central dashboard to match per-run dashboards to event logs by exact run_id instead of fragile name/timestamp heuristics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Auto-inject runtime diagnostics (PID, platform, Python version, cwd, conductor version, started_at, run_id, log_file, bg_mode) into the workflow_started event. Dashboard port/URL included when --web is active; parent_pid included in --web-bg mode. System metadata flows through: - JSONL event log (via EventLogSubscriber) - Web dashboard /api/info endpoint - Checkpoint files (for resume context) PID files are intentionally left unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jrob5756
left a comment
There was a problem hiding this comment.
Really cool. Thanks for contributing. I have a few comments. Please take a look and we can merge!
| "platform": sys.platform, | ||
| "python_version": _platform.python_version(), | ||
| "conductor_version": self._conductor_version(), | ||
| "cwd": os.getcwd(), |
There was a problem hiding this comment.
Critical: os.getcwd() can raise unhandled OSError
If the working directory is deleted between process start and this call (CI runners, containers, temp-dir cleanup), this raises FileNotFoundError/OSError. Unlike _conductor_version() which is wrapped in try/except, this method has no protection — and it's called at the top of _execute_loop(), so an unhandled exception here crashes the entire workflow before any agent runs with a confusing error.
try:
cwd = os.getcwd()
except OSError:
cwd = "<unavailable>"Or wrap the entire _build_system_metadata() body in try/except to match _conductor_version()'s pattern.
There was a problem hiding this comment.
Good catch! Wrapped os.getcwd() in try/except — falls back to '' on OSError. Matches the defensive pattern used by _conductor_version().
| "workflow_name": data.get("name", ""), | ||
| "started_at": event.get("timestamp", 0), | ||
| "metadata": data.get("metadata", {}), | ||
| "system": data.get("system", {}), |
There was a problem hiding this comment.
Critical: Unauthenticated endpoint exposes sensitive system info
The system dict contains PID, filesystem paths (cwd, log_file), platform details, and parent PID — all served via an unauthenticated HTTP GET to any network client that can reach the dashboard port.
Consider:
- Omitting
systemfrom/api/infoentirely (it's still in the event log for diagnostics) - Or limiting to non-sensitive fields only (e.g.,
conductor_version,started_at)
There was a problem hiding this comment.
Agreed — stripped the system dict from /api/info entirely. The endpoint now only returns non-sensitive fields: run_id, workflow_name, started_at, metadata, and conductor_version. Full diagnostics remain in the event log for debugging.
| # Parse --metadata key=value flags (separate from inputs) | ||
| cli_metadata: dict[str, str] = {} | ||
| if raw_metadata: | ||
| cli_metadata.update(parse_input_flags(raw_metadata)) |
There was a problem hiding this comment.
Important: parse_input_flags silently coerces metadata values
parse_input_flags calls coerce_value(), which converts "42" → int, "true" → bool, "null" → None. So --metadata work_item_id=42 silently becomes {"work_item_id": 42} (int, not string). The type annotation says dict[str, str] but actual values will be int | bool | None | list | dict.
Metadata values should stay as strings since they're opaque key-value pairs. Consider a dedicated parse_metadata_flags() that splits on first = without coercion, or add a coerce=False parameter to parse_input_flags.
There was a problem hiding this comment.
Great point — added a dedicated parse_metadata_flags() that splits on first = without any coercion. Metadata values stay as raw strings, keeping the dict[str, str] annotation honest.
| # Forward metadata | ||
| if metadata: | ||
| for key, value in metadata.items(): | ||
| cmd.extend(["--metadata", f"{key}={value}"]) |
There was a problem hiding this comment.
Important: Missing _serialize_value() — nested metadata breaks in background mode
Inputs (line 110) use _serialize_value(value) to handle non-string types (dicts, lists → JSON). Metadata uses bare f"{key}={value}". If YAML metadata contains nested dicts:
metadata:
config:
base_url: https://example.com…this produces config={'base_url': 'https://example.com'} (Python repr, not JSON), which fails to parse on the child side.
| cmd.extend(["--metadata", f"{key}={value}"]) | |
| cmd.extend(["--metadata", f"{key}={_serialize_value(value)}"]) |
There was a problem hiding this comment.
Nice find — switched to _serialize_value(value) so nested dicts/lists get proper JSON serialization instead of Python repr. Matches the pattern already used for inputs on line 110.
| web_dashboard=dashboard, | ||
| run_id=event_log_subscriber.run_id if event_log_subscriber else "", | ||
| log_file=str(event_log_subscriber.path) if event_log_subscriber else "", | ||
| dashboard_port=(dashboard._actual_port if dashboard is not None else None), |
There was a problem hiding this comment.
Important: Accessing private dashboard._actual_port
_actual_port is a private attribute. Consider adding a public @property port on WebDashboard that returns self._actual_port or self._port, then use dashboard.port here.
There was a problem hiding this comment.
Added a public port property on WebDashboard that returns _actual_port or _port. Updated run.py to use dashboard.port instead of reaching into the private attribute.
| run_id: str = "", | ||
| log_file: str = "", | ||
| dashboard_port: int | None = None, | ||
| bg_mode: bool = False, |
There was a problem hiding this comment.
Suggestion: Consider grouping informational params into a dataclass
These 4 new params (run_id, log_file, dashboard_port, bg_mode) are purely informational — not used for orchestration, only passed through to event data. The constructor already has 10 params; this brings it to 14.
Consider grouping into a dataclass:
@dataclass
class RunContext:
run_id: str = ""
log_file: str = ""
dashboard_port: int | None = None
bg_mode: bool = FalseThen pass a single run_context: RunContext | None = None parameter. This keeps the constructor clean and makes future additions trivial.
There was a problem hiding this comment.
Went ahead and did this in this pass — added a RunContext dataclass grouping run_id, log_file, dashboard_port, and bg_mode. WorkflowEngine constructor now takes a single run_context param instead of four. Should make future additions painless.
- Guard os.getcwd() with try/except OSError in _build_system_metadata() - Strip sensitive system info (PID, cwd, log_file) from /api/info endpoint - Add parse_metadata_flags() to keep metadata values as raw strings (no coercion) - Use _serialize_value() for metadata in bg_runner to handle nested dicts - Add public WebDashboard.port property, stop accessing _actual_port externally - Group informational params (run_id, log_file, dashboard_port, bg_mode) into RunContext dataclass Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Adds an optional
metadatadict to workflow configuration with two binding paths:--metadata/-mCLI flagsBoth are merged (CLI wins on conflicts) and included verbatim in the
workflow_startedevent, enabling downstream consumers to adapt behavior without parsing YAML source.Closes #106
Changes
src/conductor/config/schema.py: Addmetadata: dict[str, Any]field toWorkflowDef(empty dict default)src/conductor/engine/workflow.py: Includemetadatain theworkflow_startedevent datasrc/conductor/cli/app.py: Add--metadata/-mCLI option, parsed separately from--inputsrc/conductor/cli/run.py: Acceptmetadataparam, merge CLI metadata on top of YAML metadata after config loadsrc/conductor/cli/bg_runner.py: Forward--metadataflags to background child processExample
YAML (static)
CLI (dynamic, merged on top)
Result in event log
{ "type": "workflow_started", "data": { "name": "twig-sdlc", "metadata": { "tracker": "ado", "project_url": "https://dev.azure.com/org/Project", "work_item_id": "1814" } } }Backward Compatibility
metadatadefaults to{}— existing workflows and CLI invocations are completely unaffected--metadatais optional — omitting it changes nothingmetadatakey