fix(cmd/gc): wire ConfigDir into the prompt render context - #13
Open
austinborn wants to merge 1 commit into
Open
fix(cmd/gc): wire ConfigDir into the prompt render context#13austinborn wants to merge 1 commit into
austinborn wants to merge 1 commit into
Conversation
{{ .ConfigDir }} rendered as an empty string in every agent prompt, so a
pack-shipped helper script referenced as {{ .ConfigDir }}/assets/scripts/
<tool>.py resolved to a bare /assets/scripts/<tool>.py and exited 127.
ConfigDir was only ever populated in session-setup scope, where
SessionSetupContext carries it for command, session_setup, pre_start and
session_live expansion. Prompts take a different path: renderPrompt reads
PromptContext, flattened by buildTemplateData into a map[string]string
that never had a ConfigDir key. Because the template executes with
Option("missingkey=zero") over a map whose element type is string, the
absent key yielded "" instead of an execute-time error, which is why the
breakage stayed invisible.
Add ConfigDir to PromptContext, publish it from buildTemplateData, and
resolve it through a shared promptConfigDir helper (the agent's config
SourceDir, else the city root). All three PromptContext construction
sites and the existing session-setup path now call that helper, so the
two scopes cannot drift apart again.
TestRenderPromptConfigDir guards the regression: with the fix removed it
renders "/assets/scripts/slack.py send hi", the reported symptom exactly.
Generated by the operator's software factory.
City: factory-main · Agent: local-core.builder-3
On behalf of: @austinborn
Co-Authored-By: operator-factory-bot <factory-bot@operator-domain.invalid>
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
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.
Summary
{{ .ConfigDir }}rendered as an empty string in every agent prompt, so every pack-shipped helper script resolved to a bare/assets/scripts/<tool>.py(no directory prefix at all) and exited 127. This wiresConfigDirinto the prompt render context, which is where it wasn't.And the failure was silent. No error, nothing on stderr, just a path with its prefix quietly removed.
Which layer was wrong, and how that was established
So the prompts were right and the renderer was wrong. That's worth stating plainly, because the opposite conclusion was just as available at the start, and it would have meant editing 41 call sites instead of adding one map key.
ConfigDiris a real Gas City template variable, but it only ever lived in one scope (session setup).resolveTemplatebuilds aSessionSetupContextcarryingConfigDirand uses it to expandcommand,session_setup,pre_startandsession_live. Prompts don't go through that path at all. They go throughrenderPrompt, whose data comes fromPromptContextflattened bybuildTemplateDatainto amap[string]string, and that map never had aConfigDirkey.So the silence comes from those two facts together. The template executes with
Option("missingkey=zero")over a map whose element type isstring, so an absent key yields""(not an error) and nothing downstream can tell the difference. Had the data been a struct, the same lookup would've blown up at execute time, and this probably would have been caught years ago.flowchart TD A[agent config] --> B[resolveTemplate] B --> C[SessionSetupContext<br/>has ConfigDir] B --> D[PromptContext<br/>had no ConfigDir] C --> E[expandSessionSetup<br/>command, pre_start, session_live] D --> F[buildTemplateData<br/>map string to string] F --> G[template execute<br/>missingkey=zero] G --> H["{{ .ConfigDir }} renders empty<br/>path becomes /assets/scripts/tool.py<br/>exit 127"]So the two scopes disagreed about a variable that names the same concept in both. This change makes them share one definition, so they can't drift apart by accident.
What changed
PromptContextgains aConfigDirfield andbuildTemplateDatapublishes it (one map key, which is essentially the whole fix). A smallpromptConfigDirhelper applies the fallback (the agent's configSourceDir, else the city root), and all four sites now call it: the threePromptContextconstruction sites intemplate_resolve.go,cmd_prime.goandcmd_lint.go, plus the existing session-setup path, which previously inlined the same two-line fallback. And sharing that helper is what stops the two scopes drifting apart again.Verification
And the new
TestRenderPromptConfigDirreproduces the production symptom exactly. With the fix removed it fails with:That left-hand string is the reported bug character for character (not a paraphrase of it), so the test guards the regression rather than just exercising the new field.
TestPromptConfigDircovers both fallback branches (the pack dir, and the city-root fallback).End to end, against a real city, comparing the released binary with one built from this branch:
slack.pypathgc1.4.0/assets/scripts/slack.py<pack-cache>/packs/local-core/assets/scripts/slack.pyThe resolved file exists and runs (checked with a harmless
--help, so nothing got posted anywhere). And across the planner, architect, reviewer and builder prompts, every{{ .ConfigDir }}call site now resolves, with none left rendering bare.Test plan
go test ./cmd/gc/...passes.gc prime <agent>renders a helper-script path with a real directory prefix.Note for reviewers
Building this package on macOS needs ICU headers for a transitive CGO dependency, which fail on a stock machine on
mainas well as here.CGO_CFLAGS,CGO_CXXFLAGSandCGO_LDFLAGSpointed at$(brew --prefix icu4c)make it build. That's pre-existing and unrelated to this change.go test ./cmd/gc/...is also red on this machine, and that's pre-existing too. It reports 37 failures, all in rig, store, Dolt and city-resolution tests (TestFindCity,TestResolveBdScopeTarget*,TestBuildStores*and friends), none of which touch prompt rendering. I checked rather than assumed: running that exact set of 37 test names against pristinemainwith this change absent fails on the same 37, so the regression count from this branch is zero. CI is the authority on a clean machine.One thing this doesn't address:
missingkey=zerowill still silently swallow any other out-of-scope or misspelled variable in a prompt. This fix closes the specific hole, but the general hazard is worth a separate look.