docs: fix README Variable substitution section to match ApplyNamespace behavior (#5)#7
Conversation
WalkthroughUpdated README variable-substitution docs to describe computing namespaced keys by duplicating underscores in the namespace then appending an underscore and the variable key; clarified resolution order (UtcpClientConfig.Variables first, then environment) and adjusted examples and code sample. CI workflow paths updated to new repo layout; tests made ProcessRunner cross-platform. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Substitutor
participant Config as UtcpClientConfig.Variables
participant Env as Environment
App->>Substitutor: Substitute(inputString, namespace)
Note over Substitutor: computed_key = (namespace with "_" duplicated) + "_" + KEY
Substitutor->>Config: Lookup(computed_key)
alt found in Config
Config-->>Substitutor: value
Substitutor-->>App: return substituted string
else not found
Substitutor->>Env: Lookup(computed_key)
alt found in Env
Env-->>Substitutor: value
Substitutor-->>App: return substituted string
else not found
Substitutor-->>App: leave token unchanged (or handle defaults)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Out-of-scope changes
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
README.md (5)
104-109: Clarify scope of underscore-doubling and reference implementation name.State explicitly that underscores are doubled only in the namespace (never in the variable key), and reference the function name (e.g., ApplyNamespace) to reduce drift between docs and code.
Apply this diff:
-Namespaced variables are computed by: +Namespaced variables are computed by (ApplyNamespace): - Take the namespace and replace every `_` with `__` (double underscores) -- Append `_` and the variable key +- Append `_` and the variable key (underscores are doubled only in the namespace) -In other words: `{namespace with underscores doubled}_{KEY}`. +In other words: `{namespace_with_underscores_doubled}_{KEY}`.
110-113: ASCII-only arrows and minor style for portability.Some terminals/renderers choke on the unicode arrow. Prefer ASCII.
Apply this diff:
-Examples: -- `variableNamespace = "manual_openlibrary"`, `key = "API_KEY"` → `manual__openlibrary_API_KEY` -- `variableNamespace = "ns_with_many_parts"`, `key = "TOKEN"` → `ns__with__many__parts_TOKEN` +Examples: +- `variableNamespace = "manual_openlibrary"`, `key = "API_KEY"` -> `manual__openlibrary_API_KEY` +- `variableNamespace = "ns_with_many_parts"`, `key = "TOKEN"` -> `ns__with__many__parts_TOKEN`
114-114: Document behavior when a variable is missing and OS case-sensitivity.Readers need to know the fallback if neither source provides a value, and that env var case rules differ across OSes.
Apply this diff:
-Variables are resolved first from `UtcpClientConfig.Variables` using the namespaced key, then from environment variables using the same namespaced key. Both `$VAR` and `${VAR}` forms in strings are supported. +Variables are resolved first from `UtcpClientConfig.Variables` using the namespaced key, then from environment variables using the same namespaced key. If not found in either source, the placeholder is left unchanged. Both `$VAR` and `${VAR}` forms in strings are supported. Note: environment variable lookup is case-insensitive on Windows and case-sensitive on Linux/macOS.
120-124: Make the snippet self-contained (no extra using required).Fully qualify UtcpClientConfig to avoid missing
using Utcp.Core.Models;in copy-paste.Apply this diff:
-var cfg = new UtcpClientConfig { ToolRepository = repo, ToolSearchStrategy = search, Variables = new(){ ["manual__openlibrary_API_KEY"] = "123" } }; +var cfg = new Utcp.Core.Models.UtcpClientConfig { ToolRepository = repo, ToolSearchStrategy = search, Variables = new(){ ["manual__openlibrary_API_KEY"] = "123" } };
120-124: Optional: Add an env-var example for parity with the text above.Show how to set the namespaced key via env for both Bash and PowerShell.
Add after the snippet:
// Alternatively, via environment variables: // Bash/Zsh: export manual__openlibrary_API_KEY=123 // PowerShell: $env:manual__openlibrary_API_KEY = '123'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~110-~110: There might be a mistake here.
Context: ... underscores doubled}_{KEY}. Examples: - variableNamespace = "manual_openlibrary", key = "API_KEY"→manual__openlibra...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...nual_openlibrary", key = "API_KEY"→manual__openlibrary_API_KEY-variableNamespace = "ns_with_many_parts", key = "TOKEN"→ns__with__many__par...
(QB_NEW_EN)
🔇 Additional comments (1)
README.md (1)
104-114: Docs match ApplyNamespace implementation
Verified thatApplyNamespacereplaces each_with__(via an internal"!"placeholder), then appends_+key, preserves casing/trimming, and that unresolved${VAR}/$VARplaceholders are returned unchanged when no matching variable or environment entry exists.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/build.yml (2)
25-37: Avoid duplicate packing across OSes; simplify to a single pack and upload artifactsRight now each matrix OS packs the projects, producing duplicates. Prefer packing once (e.g., ubuntu-latest) and pack via the solution to reduce duplication and maintenance.
Apply this to consolidate packing:
- - name: Pack Core - run: dotnet pack src/Utcp.Core/Utcp.Core.csproj -c Release --no-build -o artifacts - - name: Pack Http - run: dotnet pack src/Utcp.Http/Utcp.Http.csproj -c Release --no-build -o artifacts - - name: Pack Cli - run: dotnet pack src/Utcp.Cli/Utcp.Cli.csproj -c Release --no-build -o artifacts - - name: Pack Text - run: dotnet pack src/Utcp.Text/Utcp.Text.csproj -c Release --no-build -o artifacts - - name: Pack Mcp - run: dotnet pack src/Utcp.Mcp/Utcp.Mcp.csproj -c Release --no-build -o artifacts - - name: Pack Socket - run: dotnet pack src/Utcp.Socket/Utcp.Socket.csproj -c Release --no-build -o artifacts - - name: Pack Gql - run: dotnet pack src/Utcp.Gql/Utcp.Gql.csproj -c Release --no-build -o artifacts + - name: Pack (Solution) + if: matrix.os == 'ubuntu-latest' + run: dotnet pack Utcp.sln -c Release --no-build -o artifactsThen add an upload step (new step) to persist the packages:
- name: Upload packages if: matrix.os == 'ubuntu-latest' uses: actions/upload-artifact@v4 with: name: nupkgs path: artifacts/**/*.nupkgIf you prefer to keep per-project packing, at least gate them to one OS:
- - name: Pack Core + - name: Pack Core + if: matrix.os == 'ubuntu-latest' run: dotnet pack src/Utcp.Core/Utcp.Core.csproj -c Release --no-build -o artifacts
19-23: Utcp.sln exists at repo root; consider adding a NuGet cache step to speed up restores- uses: actions/setup-dotnet@v4 with: dotnet-version: '8.0.x' + - name: Cache NuGet + uses: actions/cache@v4 + with: + path: ~/.nuget/packages + key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json', '**/*.csproj', '**/*.sln') }} + restore-keys: | + ${{ runner.os }}-nuget-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/build.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test-pack (windows-latest)
- GitHub Check: build-test-pack (windows-latest)
🔇 Additional comments (1)
.github/workflows/build.yml (1)
25-37: Paths sanity-check passed
All referenced.csprojfiles exist undersrc/; no workflow edits needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/Utcp.Cli.Tests/ProcessRunnerTests.cs (2)
15-26: Harden command resolution on Windows; drop redundant property; trim only EOLs.
- Use ComSpec to resolve cmd path and add /D to disable AutoRun.
- CallTemplateType is already set in CliCallTemplate() (constructor), so the explicit assignment is redundant.
- TrimEnd CR/LF to avoid masking leading spaces in output.
Apply this diff:
- var template = new CliCallTemplate + var cmd = isWindows ? (System.Environment.GetEnvironmentVariable("ComSpec") ?? "cmd.exe") : null; + var template = new CliCallTemplate { - CallTemplateType = "cli", Name = "manual", - Command = isWindows ? "cmd" : "/bin/echo", - Args = isWindows ? new []{"/c", "echo", "hello"} : new []{"hello"}, + Command = isWindows ? cmd! : "/bin/echo", + Args = isWindows ? new []{"/D", "/c", "echo", "hello"} : new []{"hello"}, }; @@ - stdOut.Trim().Should().Be("hello"); + stdOut.TrimEnd('\r', '\n').Should().Be("hello");
14-14: Use OperatingSystem.IsWindows()
Target frameworks include .NET 5+ so you can simplify the check and drop the extra RuntimeInformation import by callingSystem.OperatingSystem.IsWindows()instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/Utcp.Cli.Tests/ProcessRunnerTests.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Utcp.Cli.Tests/ProcessRunnerTests.cs (1)
src/Utcp.Cli/CliCallTemplate.cs (1)
CliCallTemplate(17-21)
🔇 Additional comments (1)
tests/Utcp.Cli.Tests/ProcessRunnerTests.cs (1)
7-7: Import is appropriate for cross-platform OS checks.
No issues; needed for RuntimeInformation/OSPlatform.
1 participant
This PR updates the README Variable substitution section to reflect the actual namespacing logic implemented in DefaultVariableSubstitutor.ApplyNamespace.\n\nFixes #5.
Summary by CodeRabbit
Documentation
Chores
Tests