Uh oh!
There was an error while loading. Please reload this page.
feat(gates): markdown rendering and auto-linkification in human gate prompts - #131
Conversation
- Terminal gates: wrap prompt text in Rich Markdown for proper heading,
bold, list, and link rendering in the CLI
- Web dashboard: add GET /api/files/{path} endpoint to serve local files
relative to the workflow root with security guards (path traversal
protection, extension allowlist, 1MB size limit)
- Web dashboard: add FileViewer modal component for viewing linked files
with Markdown rendering support
- Web dashboard: intercept relative links in gate prompts to open the
FileViewer instead of navigating away; external URLs still open in
new tabs
- Tests: 13 file API security tests + 3 gate Markdown rendering tests
(79 total tests pass across both test suites)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>Add a markdown-aware post-processor that converts bare file paths and URLs in rendered gate prompts into clickable markdown links. This fixes the issue where Jinja2-rendered file paths appeared as plain grey text in the web dashboard instead of interactive file links. Processing: - Normalizes Jinja2 whitespace artifacts (collapses excessive blank lines) - Auto-detects bare http(s):// URLs and wraps in markdown link syntax - Auto-detects relative file paths (verified against workflow root), wraps in markdown links that the dashboard FileViewer can open - Markdown-aware: skips fenced code blocks, inline code, existing links - Reuses the server's extension allowlist for file detection - Security: path traversal outside workflow root is blocked Applied to both web dashboard (gate_presented event) and terminal (Rich markdown rendering in HumanGateHandler). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GateDetail's PromptMarkdown component was missing table/th/td handlers, causing markdown tables to render as raw pipe-separated text. Added the same table component overrides already used in FileViewer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ReactMarkdown requires the remark-gfm plugin to parse GitHub Flavored Markdown extensions like tables. Without it, pipe-delimited tables render as plain text inside paragraph elements. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Documents the markdown formatting support in gate prompts including Rich terminal rendering, GFM table support, auto-linkification of file paths and URLs, and the web dashboard interactive features. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 #131 +/- ##
=======================================
Coverage ? 85.73% =======================================
Files ? 56 Lines ? 8144 Branches ? 0 =======================================
Hits ? 6982 Misses ? 1162 Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Jason Robert (jrob5756)
left a comment
There was a problem hiding this comment.
A few comments before we merge.
| try: | ||
| candidate = (base_dir / stripped).resolve() | ||
| # Security: must be within base_dir | ||
| if not str(candidate).startswith(str(base_dir.resolve())): |
There was a problem hiding this comment.
Important — fragile string-prefix containment check.
ifnotstr(candidate).startswith(str(base_dir.resolve())):This has the classic prefix-confusion bug: /foo/bar is a string prefix of /foo/bar2. The new server.py (line ~290) correctly uses target.relative_to(self._workflow_root) in a try/except ValueError — please apply the same pattern here, or use candidate.is_relative_to(base_dir.resolve()) (stdlib since 3.9; project requires 3.12+).
Not exploitable in practice today because candidate is also resolved and gate roots don't usually have sibling-prefix names, but it's a footgun worth removing.
| ".ps1", | ||
| ".plan.md", | ||
| } | ||
| ) |
There was a problem hiding this comment.
Important — extension allowlist already drifting between modules.
This LINKABLE_EXTENSIONS (21 entries, includes .plan.md) and web/server.py's _FILE_ALLOWED_EXTENSIONS (20 entries, missing .plan.md) are intended to stay in sync — the docstring above even says so. They've already drifted on this very PR.
Suggest: have server.py import this constant directly, e.g. from conductor.executor.linkify import LINKABLE_EXTENSIONS.
Side note: the .plan.md entry here is dead anyway — Path.suffix returns only .md for foo.plan.md, so it's already covered by the .md entry in server.py. Consolidating to one source of truth makes this kind of confusion go away.
| r"(?!https?://)" # not a URL | ||
| r"(?:[a-zA-Z0-9_.][a-zA-Z0-9_./-]*[a-zA-Z0-9_]" # path chars with at least one / | ||
| r")" | ||
| ) |
There was a problem hiding this comment.
Suggestion — dead regex.
_FILE_PATH_RE is defined here but never referenced (grep -rn _FILE_PATH_RE src/ tests/ returns only this definition). The actual file-path linkification uses re.split(r"(\s+)", segment) token-by-token in _try_linkify_path. Safe to delete (along with the explanatory comment block immediately above).
| def _has_linkable_extension(path: str) -> bool: | ||
| """Check if a path ends with a known linkable extension.""" | ||
| lower = path.lower() | ||
| return any(lower.endswith(ext) for ext in LINKABLE_EXTENSIONS) |
There was a problem hiding this comment.
Nit — str.endswith accepts a tuple natively.
def_has_linkable_extension(path: str) ->bool:
lower=path.lower()
returnany(lower.endswith(ext) forextinLINKABLE_EXTENSIONS)Can be:
_LINKABLE_SUFFIXES=tuple(LINKABLE_EXTENSIONS)
...
def_has_linkable_extension(path: str) ->bool:
returnpath.lower().endswith(_LINKABLE_SUFFIXES)No Python-level loop, slightly faster, more idiomatic.
| # Build markdown link with forward slashes (for dashboard API) | ||
| link_target = normalized | ||
| return f"{prefix}[{stripped}]({link_target}){suffix}" |
There was a problem hiding this comment.
Nit — dead local.
link_target=normalizedreturnf"{prefix}[{stripped}]({link_target}){suffix}"Inline normalized directly into the f-string and drop the link_target line.
| return JSONResponse( | ||
| {"error": "Absolute paths are not allowed"}, | ||
| status_code=403, | ||
| ) |
There was a problem hiding this comment.
Nit — hand-rolled absolute-path detection.
The combined absolute / UNC / drive-letter check could be PurePosixPath(file_path).is_absolute() or PureWindowsPath(file_path).is_absolute() (or PurePath). Keep the "://" scheme check separate. Reduces a brittle hand-rolled check to a documented stdlib call.
| # If we bail early when ``has_connections()`` is False, a later click | ||
| # in the dashboard pushes a message to ``_gate_response_queue`` that | ||
| # nobody is awaiting, and the workflow hangs forever. | ||
| gate_base = Path(self.workflow_path).resolve().parent if self.workflow_path else None |
There was a problem hiding this comment.
Suggestion — triplicated gate_base derivation.
The same expression appears at lines 834, 843, and 1433-1436 (and a similar pattern at line 583 for workflow_dir):
gate_base=Path(self.workflow_path).resolve().parentifself.workflow_pathelseNoneExtract a cached property:
@propertydef_workflow_dir(self) ->Path|None:
returnPath(self.workflow_path).resolve().parentifself.workflow_pathelseNoneand reuse at all four call sites.
Merged upstream/main into feat/gate-markdown-rendering. The only conflict was in src/conductor/web/static/index.html (competing built-asset hashes). Resolved by rebuilding the frontend to produce a single bundle that includes both the gate-markdown-rendering features and upstream's breadcrumb/subworkflow changes. Fixed TypeScript errors from the merge in GroupDetail, GateNode, WorkflowGraph, WorkflowNode, YamlViewer, and use-viewed-context. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Review feedback addressed: 1. linkify.py: Use is_relative_to() for path containment check instead of fragile string-prefix comparison (resolved via upstream merge). 2. linkify.py: Remove dead .plan.md entry from LINKABLE_EXTENSIONS (already covered by .md via Path.suffix). server.py now imports LINKABLE_EXTENSIONS from linkify — single source of truth. 3. linkify.py: Remove dead _FILE_PATH_RE regex (never referenced). 4. linkify.py: Use tuple str.endswith() idiom for _has_linkable_extension (no Python-level loop, more idiomatic). 5. linkify.py: Inline dead link_target local into the f-string. 6. server.py: Replace hand-rolled absolute-path detection with PurePosixPath/PureWindowsPath.is_absolute() (stdlib, documented). 7. workflow.py: Extract _workflow_dir cached property to eliminate triplicated gate_base/gate_base_dir derivation at three call sites. Also merged upstream/main and rebuilt frontend assets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PolyphonyRequiem
commented
May 5, 2026
Addressed all 7 review comments in commit 6c37af1:
All 87 linkify+server tests and 517 engine tests pass. |
- tests/test_web/test_server.py: union imports (Path + traceback/patch both now needed) - src/conductor/web/static/index.html and assets/: regenerated via npm run build (new hashes: index-ZAvmqxaV.js, index-CPAEvQwD.css) - frontend/tsconfig.tsbuildinfo: regenerated by tsc -b Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
…, #121-#123, #125, #129, #130, #131, #139, #141-#144, #146) CHANGELOG: add 6 newer PRs (#119, #121, #122, #123, #125, #113, #130, #131, #141, #146) to [Unreleased] alongside the previously documented batch. docs/workflow-syntax.md: - Add metadata + instructions fields to the workflow configuration block. - Add input_mapping and max_depth to Sub-Workflow Steps; correct stale claims that circular references are rejected and that workflow steps cannot be used in for_each groups. - Add 'Sub-workflows in for_each groups' subsection with example. - Add JSON stdout auto-parsing note + example to Script Steps output section. - Add type-appropriate zero values table to Workflow Inputs. - Add 'Workflow Metadata Variables' subsection covering workflow.dir, workflow.file, workflow.name. - Update on_start hook context list to include the new workflow.dir/file vars. docs/cli-reference.md: - Document --metadata/-m, --workspace-instructions, and --instructions flags on conductor run. - Add 'Metadata and Instructions' examples block. - Update conductor validate to describe the new template-reference error/warning checks added in #125. docs/providers/claude.md, docs/providers/comparison.md: - Replace stale 'All models support a 200K token context window' / '200K (all models)' claims with notes that the dashboard now sources context_window_max from each provider's SDK at runtime (#144). README.md: - Refresh the Features list to mention sub-workflow composition, dialog mode, workspace instructions, breadcrumb navigation, and the enhanced validate behavior. - Add --metadata, --workspace-instructions, --instructions to the conductor run options table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs: changelog + doc updates for unreleased PRs (#100, #109-#111, #119, #121-#123, #125, #129, #130, #131, #139, #141-#144, #146) CHANGELOG: add 6 newer PRs (#119, #121, #122, #123, #125, #113, #130, #131, #141, #146) to [Unreleased] alongside the previously documented batch. docs/workflow-syntax.md: - Add metadata + instructions fields to the workflow configuration block. - Add input_mapping and max_depth to Sub-Workflow Steps; correct stale claims that circular references are rejected and that workflow steps cannot be used in for_each groups. - Add 'Sub-workflows in for_each groups' subsection with example. - Add JSON stdout auto-parsing note + example to Script Steps output section. - Add type-appropriate zero values table to Workflow Inputs. - Add 'Workflow Metadata Variables' subsection covering workflow.dir, workflow.file, workflow.name. - Update on_start hook context list to include the new workflow.dir/file vars. docs/cli-reference.md: - Document --metadata/-m, --workspace-instructions, and --instructions flags on conductor run. - Add 'Metadata and Instructions' examples block. - Update conductor validate to describe the new template-reference error/warning checks added in #125. docs/providers/claude.md, docs/providers/comparison.md: - Replace stale 'All models support a 200K token context window' / '200K (all models)' claims with notes that the dashboard now sources context_window_max from each provider's SDK at runtime (#144). README.md: - Refresh the Features list to mention sub-workflow composition, dialog mode, workspace instructions, breadcrumb navigation, and the enhanced validate behavior. - Add --metadata, --workspace-instructions, --instructions to the conductor run options table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: bump version to 0.1.11 and changelog #148 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Adds full Markdown rendering support to human gate prompts in both the terminal CLI and the web dashboard.
Changes
Rich Terminal Rendering
Auto-Linkification (\linkify.py)
Web Dashboard: File Viewing
Web Dashboard: GFM Table Support
emark-gfm\ plugin for GitHub Flavored Markdown table parsing
Documentation
Tests