-
Couldn't load subscription status.
- Fork 3.5k
Fix: Pass input parameter to extension calls with {{input}} in patterns #1799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add README-BUG-FIX.MD with documentation for extension bug fix process - Add test_results_with_coverage_before_fix.txt with baseline test coverage - Add word-generator-counter.sh script for updating word-generator extension - Add fabric-development.code-workspace for VS Code workspace configuration - Update go.sum with latest dependencies
…on validation & new tests - Refactor ApplyTemplate into single-pass token resolver (plugins + extensions) - Add input sentinel replacement inside extension values (fix TestSentinelTokenReplacement) - Add validation in extension registry (cmd_template required) - Standardize debug logs & rename op/val -> operation/value; introduce matchTriple helper - Add mixed input+variable and multiple extension input tests - Fix test scripts ($ issues) and timeout units (5s) - Add race-detector run (no races) and coverage run (template ~69.7%) - Update VS Code tasks/launch for debugging All tests passing (go test ./...); coverage collected with -cover.
- Organized debugging documentation with other bug fix materials - Keeps all extension debugging resources in one location
Root cause: The applyVariables() function in patterns.go was calling
ApplyTemplate() with an empty string as the input parameter, causing
extension calls with {{input}} in their value to receive empty values.
The fix: Change line 104 from:
template.ApplyTemplate(withSentinel, variables, "")
to:
template.ApplyTemplate(withSentinel, variables, input)
This ensures that when extensions use {{input}} within their value
parameter (e.g., {{ext:openai:chat:{{input}}}}), the sentinel token
gets replaced with the actual user input during template processing.
Testing:
- All existing tests pass (TestSentinelTokenReplacement, etc.)
- Real-world test now works: echo "text" | fabric -p ai_echo
- Extension now receives actual input instead of empty string
Also includes:
- Added debug output to show input parameter in ApplyTemplate
- Updated README-BUG-FIX.MD with detailed explanation
- Added setup_fabric_llms_attempt_two.sh for OpenAI extension testing
Renamed files to follow consistent kebab-case naming convention: - README-BUG-FIX.MD → input-extension-bug-fix.md - debug_instructions.md → debugging-extensions.md Added bidirectional references: - input-extension-bug-fix.md now links to debugging-extensions.md - debugging-extensions.md now links to input-extension-bug-fix.md This helps readers navigate between the specific bug fix documentation and the general debugging guide. File names are now more descriptive and follow Fabric's documentation naming conventions.
Added test_results_with_coverage_after_fix.txt showing: - All 468 tests pass (13 new tests added) - New sentinel token tests validate the fix: * TestSentinelTokenReplacement (4 subtests) * TestSentinelInVariableProcessing (3 subtests) * TestExtensionValueWithSentinel * TestNestedInputInExtension * TestMultipleExtensionsWithInput * TestExtensionValueMixedInputAndVariable Comparison with before fix: - Before: 455 tests - After: 468 tests (+13) - All tests pass, proving the bug fix works correctly
Created PR-SUBMISSION.md with complete PR information: - Problem description and examples - Root cause analysis - Solution with code changes - Testing evidence (before/after) - Test statistics (455→468 tests, +13 new) - Files changed summary - Submission instructions for multiple methods Ready for PR submission to danielmiessler/Fabric.
Enhanced setup_openai_extension_test.sh to demonstrate dual-LLM pattern: - Added Claude/Anthropic extension alongside OpenAI - Updated ai_echo pattern to call both OpenAI and Claude - Added ANTHROPIC_API_KEY validation - Added ANTHROPIC_API_BASE_URL with default - Shows how extensions enable AI consensus patterns Pattern now demonstrates the bug fix with real-world multi-AI use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Fixes incorrect handling of {{input}} when nested inside extension calls within patterns so extensions receive the actual user input. Also streamlines template processing and adds comprehensive tests and docs.
- Pass the actual input into template processing in patterns.go so extension values with {{input}} are correctly substituted
- Refactor ApplyTemplate to support a sentinel mechanism and add debug logging; add tests covering various extension/input combinations
- Add documentation and helper scripts; introduce VS Code config files
Reviewed Changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/plugins/db/fsdb/patterns.go | Core one-line fix: pass input to ApplyTemplate so sentinel replacement resolves to actual input within extension values |
| internal/plugins/template/template.go | Introduces inputSentinel, consolidates token processing, replaces sentinel within extension values, and adds debug logging |
| internal/plugins/template/template_sentinel_test.go | Adds comprehensive tests for sentinel handling, nested {{input}} in extensions, and variable processing |
| internal/plugins/template/template_extension_multiple_test.go | Adds test covering multiple extension calls with {{input}} |
| internal/plugins/template/template_extension_mixed_test.go | Adds test for mixing {{input}} with other variables inside an extension value |
| internal/plugins/template/extension_registry.go | Adds a validation hook for extension definitions during registration |
| bug-fix-extensions/* | Adds docs, scripts, and test result artifacts related to the bug and its verification |
| fabric-development.code-workspace | Adds VS Code workspace with absolute local paths (environment-specific) |
| .vscode/* | Adds editor debug/build configs |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| var missingVars []string | ||
| r := regexp.MustCompile(`\{\{([^{}]+)\}\}`) | ||
| debugf("Starting template processing with input='%s'\n", input) |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug logging prints the full user input, which can leak sensitive data at trace level. Consider masking or truncating the input (e.g., log only length or first N characters) or guard this log behind a higher log level flag.
| debugf("Starting template processing with input='%s'\n", input) | |
| const maxPreview = 10 | |
| preview := input | |
| if len(input) > maxPreview { | |
| preview = input[:maxPreview] + "..." | |
| } | |
| debugf("Starting template processing with input (len=%d, preview='%s')\n", len(input), preview) |
| err = extensionManager.RegisterExtension(configPath) | ||
| if err != nil { | ||
| t.Fatalf("Failed to register extension: %v", err) |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests create and execute shell scripts and may fail on Windows or restricted CI environments. Add an early skip for unsupported OS or permission errors (e.g., if runtime.GOOS == "windows" or if the error indicates exec not permitted) to keep the test suite portable.
| mgr := NewExtensionManager(configDir) | ||
| if err := mgr.RegisterExtension(filepath.Join(configsDir, "multi-echo.yaml")); err != nil { | ||
| t.Fatalf("register: %v", err) | ||
| } |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like the other extension tests, this assumes a POSIX shell and executable permissions. Add a platform/permissions guard (skip on Windows or when execution is not permitted) to avoid CI flakiness.
| // TestExtensionValueMixedInputAndVariable ensures an extension value mixing {{input}} and another template variable is processed. | ||
| func TestExtensionValueMixedInputAndVariable(t *testing.T) { | ||
| input := "PRIMARY" | ||
| variables := map[string]string{ | ||
| "suffix": "SUF", | ||
| } |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To improve portability, add an early skip for Windows (runtime.GOOS == "windows") before creating and invoking shell scripts; this complements the existing skip for 'operation not permitted'.
| "path": "/Users/ourdecisions/devHelp/Fabric-fix", | ||
| "name": "Fabric-fix" | ||
| }, | ||
| { | ||
| "path": "/Users/ourdecisions/.config/fabric", |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This workspace file contains absolute, user-specific paths. Recommend removing this file from the repo or replacing absolute paths with relative or templated values to avoid environment-specific breakage.
| "path": "/Users/ourdecisions/devHelp/Fabric-fix", | |
| "name": "Fabric-fix" | |
| }, | |
| { | |
| "path": "/Users/ourdecisions/.config/fabric", | |
| "path": ".", | |
| "name": "Fabric-fix" | |
| }, | |
| { | |
| "path": "./fabric-config", |
| echo "Removing existing word-generator extension..." | ||
| fabric --rmextension=word-generator | ||
|
|
||
| echo "Copying updated word-generator configuration..." | ||
| cp /Users/ourdecisions/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.yaml /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | ||
|
|
||
| echo "Copying updated word-generator script..." | ||
| cp /Users/ourdecisions/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.py /Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py | ||
|
|
||
| fabric --addextension /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | ||
|
|
||
| # Update the text of path of /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | ||
| # of executable: /usr/local/bin/word-generator.py to /Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py | ||
| sed -i '' 's|/usr/local/bin/word-generator.py|/Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py|g' /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Script hard-codes absolute user paths and uses macOS-specific sed (-i ''). Parameterize with $HOME and detect sed flavor for portability (BSD vs GNU). Consider moving this environment-specific helper out of the repo or document it as example-only.
| echo "Removing existing word-generator extension..." | |
| fabric --rmextension=word-generator | |
| echo "Copying updated word-generator configuration..." | |
| cp /Users/ourdecisions/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.yaml /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | |
| echo "Copying updated word-generator script..." | |
| cp /Users/ourdecisions/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.py /Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py | |
| fabric --addextension /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | |
| # Update the text of path of /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | |
| # of executable: /usr/local/bin/word-generator.py to /Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py | |
| sed -i '' 's|/usr/local/bin/word-generator.py|/Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py|g' /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | |
| # Detect sed flavor for portability (BSD/macOS vs GNU/Linux) | |
| if sed --version >/dev/null 2>&1; then | |
| # GNU sed | |
| SED_INPLACE="sed -i" | |
| else | |
| # BSD/macOS sed | |
| SED_INPLACE="sed -i ''" | |
| fi | |
| echo "Removing existing word-generator extension..." | |
| fabric --rmextension=word-generator | |
| echo "Copying updated word-generator configuration..." | |
| cp "$HOME/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.yaml" "$HOME/.config/fabric/extensions/configs/word-generator.yaml" | |
| echo "Copying updated word-generator script..." | |
| cp "$HOME/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.py" "$HOME/.config/fabric/extensions/bin/word-generator.py" | |
| fabric --addextension "$HOME/.config/fabric/extensions/configs/word-generator.yaml" | |
| # Update the text of path of $HOME/.config/fabric/extensions/configs/word-generator.yaml | |
| # of executable: /usr/local/bin/word-generator.py to $HOME/.config/fabric/extensions/bin/word-generator.py | |
| $SED_INPLACE 's|/usr/local/bin/word-generator.py|'"$HOME"'/.config/fabric/extensions/bin/word-generator.py|g' "$HOME/.config/fabric/extensions/configs/word-generator.yaml" |
| cp /Users/ourdecisions/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.yaml /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | ||
|
|
||
| echo "Copying updated word-generator script..." | ||
| cp /Users/ourdecisions/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.py /Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py | ||
|
|
||
| fabric --addextension /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | ||
|
|
||
| # Update the text of path of /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | ||
| # of executable: /usr/local/bin/word-generator.py to /Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py | ||
| sed -i '' 's|/usr/local/bin/word-generator.py|/Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py|g' /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Script hard-codes absolute user paths and uses macOS-specific sed (-i ''). Parameterize with $HOME and detect sed flavor for portability (BSD vs GNU). Consider moving this environment-specific helper out of the repo or document it as example-only.
| cp /Users/ourdecisions/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.yaml /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | |
| echo "Copying updated word-generator script..." | |
| cp /Users/ourdecisions/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.py /Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py | |
| fabric --addextension /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | |
| # Update the text of path of /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | |
| # of executable: /usr/local/bin/word-generator.py to /Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py | |
| sed -i '' 's|/usr/local/bin/word-generator.py|/Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py|g' /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | |
| cp "$HOME/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.yaml" "$HOME/.config/fabric/extensions/configs/word-generator.yaml" | |
| echo "Copying updated word-generator script..." | |
| cp "$HOME/devHelp/Fabric-fix/internal/plugins/template/Examples/word-generator.py" "$HOME/.config/fabric/extensions/bin/word-generator.py" | |
| fabric --addextension "$HOME/.config/fabric/extensions/configs/word-generator.yaml" | |
| # Update the text of path of /Users/ourdecisions/.config/fabric/extensions/configs/word-generator.yaml | |
| # of executable: /usr/local/bin/word-generator.py to /Users/ourdecisions/.config/fabric/extensions/bin/word-generator.py | |
| # Detect sed flavor (BSD/macOS vs GNU) | |
| if sed --version >/dev/null 2>&1; then | |
| # GNU sed | |
| SED_INPLACE=(-i) | |
| else | |
| # BSD/macOS sed | |
| SED_INPLACE=(-i '') | |
| fi | |
| sed "${SED_INPLACE[@]}" "s|/usr/local/bin/word-generator.py|$HOME/.config/fabric/extensions/bin/word-generator.py|g" "$HOME/.config/fabric/extensions/configs/word-generator.yaml" |
| github.com/danielmiessler/fabric/cmd/code_helper coverage: 0.0% of statements | ||
| github.com/danielmiessler/fabric/cmd/fabric coverage: 0.0% of statements | ||
| github.com/danielmiessler/fabric/cmd/generate_changelog coverage: 0.0% of statements | ||
| github.com/danielmiessler/fabric/cmd/generate_changelog/internal coverage: 0.0% of statements |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generated test result artifacts should not be committed; they add noise and can quickly become stale. Recommend removing this file (and the corresponding 'after' file) and adding them to .gitignore.
| github.com/danielmiessler/fabric/cmd/code_helper coverage: 0.0% of statements | ||
| github.com/danielmiessler/fabric/cmd/fabric coverage: 0.0% of statements | ||
| github.com/danielmiessler/fabric/cmd/generate_changelog coverage: 0.0% of statements | ||
| github.com/danielmiessler/fabric/cmd/generate_changelog/internal coverage: 0.0% of statements |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the 'before' coverage file: this looks like an ephemeral build artifact. Consider removing it from version control and ignoring via .gitignore.
| ## The Fix | ||
| **File:** `/Users/ourdecisions/devHelp/Fabric-fix/internal/plugins/db/fsdb/patterns.go` |
Copilot
AI
Oct 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation includes absolute, user-specific file system paths. Replace with repository-relative paths (e.g., internal/plugins/db/fsdb/patterns.go) or generic placeholders to keep docs portable.
| **File:** `/Users/ourdecisions/devHelp/Fabric-fix/internal/plugins/db/fsdb/patterns.go` | |
| **File:** `internal/plugins/db/fsdb/patterns.go` |
|
@danielmiessler, I have long admired your work in https://danielmiessler.com/blog/fabric-origin-story, contributions to OWASP, and to the tech community. I want to help others reach Human 3.0. @ksylvan Thank you also for your contributions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nickarino Reviewing this was like trying to fix LLM-generated code.
Please remove all the AI-generated interim docs and human-review this yourself. Then, please re-submit the PR.
It should be small, focused, and easy for a human to review
Also, please follow the instructions in the CONTRIBUTING docs for how to generate the changelog entry (and squash all commits into one commit).
| @@ -0,0 +1,77 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated. Remove all the vscode additions, please.
| @@ -0,0 +1,113 @@ | |||
| # Debugging Fabric Extensions | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also unrelated. Was this (and the PR) generated by an LLM?
| @@ -0,0 +1,165 @@ | |||
|
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, this seems like it's auxiliary docs which you used to giude Claude code or some other LLM coding assistant. Please remove.
| @@ -0,0 +1,267 @@ | |||
| # Pull Request: Fix input parameter bug in extension calls | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More LLM coding detritus... Please remove.
| @@ -0,0 +1,216 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems largely unnecessary. Use the already established patterns in the codebase for tests, please.
| @@ -0,0 +1,1137 @@ | |||
| github.com/danielmiessler/fabric/cmd/code_helper coverage: 0.0% of statements | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole bug-extensions/ directory seems filled with AI slop and test-coverage text files. I need pull requests to be focused and small.
| @@ -0,0 +1,26 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this too.
|
@nickarino I appreciate the contribution, and I want to make sure we're not adding thousands of lines of unnecessary interim docs and scripts to a PR that should be concise, focused, and easy to understand and maintain. Also, please keep in mind that Fabric is international, so use the existing internationalization patterns if adding strings that the user will see. |
- Add 'fabric' binary to .gitignore (built by integration tests) - Simplify .vscode/launch.json to only unit test debugging - Integration tests moved to separate Fabric-integration project
- Added Fabric-integration project to multi-folder workspace - Enables easy switching between main development and integration testing
|
@ksylvan No worries. I will remove the information that I thought would bring clarity and follow the instructions in the CONTRIBUTING docs for how to generate the changelog entry (and squash all commits into one commit), and check existing internationalization patterns. Then, I will submit a new PR. |
|
Closing in favor of PR #1802 which follows contributing guidelines properly |
Problem
When using
{{input}}inside extension calls within patterns (e.g.,{{ext:openai:chat:{{input}}}}), the input parameter was not being passed through correctly, resulting in extensions receiving empty strings.Solution
Fixed a 1-line bug in
/internal/plugins/db/fsdb/patterns.goline 104 whereapplyVariables()was passing an empty string""instead of the actualinputparameter toApplyTemplate().Changed:
To:
Testing
{{input}}now work correctly-v=input:flag - doesn't work)Documentation
📁 Complete documentation is in the
bug-fix-extensions/folder:Impact
This fix enables extensions to receive user input via
{{input}}, unlocking powerful patterns like:Without this fix, extensions could not access user input at all when called from patterns.