cli: StandardFlags helper for downstream agents (closes #94) by erain · Pull Request #101 · erain/glue

erain · 2026-05-07T10:59:41Z

Summary

New thin package github.com/erain/glue/cli with StandardConfig, StandardFlagDefaults, and RegisterStandardFlags(fs, defaults) func() StandardConfig.
Wires --provider, --model, --id, --store, --work, --max-turns onto a caller-supplied flag.FlagSet.
Per-flag defaults are overridable; help text mentions failover semantics so agents don't need to redocument them.
README "CLI" section gains a "Standard flags for downstream agents" subsection.

cmd/glue remains the canonical CLI runner. This is shared infrastructure for downstream agent binaries (e.g. agents/glue-review, future agents).

Stacked on #95–#100. Closes #94.

Test plan

go build ./...
go vet ./...
go test ./cli -count=1 (3/3 — defaults round-trip, argv parse, partial-defaults override)

🤖 Generated with Claude Code

Every Glue agent re-implemented the same per-tool boilerplate: a json.Unmarshal of call.Arguments into a private struct, plus local textResult / errorResult helpers. Promote the pattern into the framework so tool definitions drop ~15 lines each and gain consistent malformed- argument handling. - glue.TextResult(string) and glue.ErrorResult(error) replace the duplicated helpers (the agent had its own copies; future agents would too). - glue.NewTool[T any](spec, fn) decodes ToolCall.Arguments into T, treats empty arguments as the zero value, and returns an error ToolResult on JSON decode failure so the loop never crashes on a malformed call. Migrate agents/glue-review/tools.go as the first internal consumer: three tools converted, the local textResult/errorResult helpers removed. Schema generation from T is intentionally out of scope; callers continue to pass Parameters explicitly. Verification: go build ./... # ok go vet ./... # ok go test ./... # ok (all packages green) Closes #88 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions

Summary

This branch adds framework-level conveniences for multi-provider agent deployments: typed tool construction (NewTool[T]), event helpers (WithStreamWriter, WithToolLogger), provider failover (WithFailover), a driver-style provider registry, standard CLI flags, versioned prompt catalogs, and standalone tools/fs and tools/git packages. It also adds StopReasonMaxTurns to distinguish budget exhaustion from natural stops.

Issues

[major] failover.go:71 — The proxy goroutine in failoverProvider.Stream doesn't handle ctx cancellation when forwarding events from the committed provider. If ctx is canceled after out <- first but before the for ev := range ch loop finishes, the goroutine leaks while blocked reading ch or writing to out (unbounded 16-buffer might full-block, consumers might stop draining). Fix: In failover.go, wrap the forwarding goroutine to select on ctx.Done() in addition to the for ev := range ch loop, and close out on context cancellation.
[major] failover.go:88 — return nil, &FailoverError{Attempts: attempts} leaks channels for the successfully Streamed providers that were discarded before successful first-event commit. Fix: In failover.go, drain the ch returned from p.Stream in each continue fallback path (stream error, empty stream, first-event error), and separately drain the ch from every prior iteration of the loop that was never selected, to prevent goroutine leaks from unclosed upstream channels.
[major] tools/fs/read.go:68 — The local variable cap shadows the builtin cap function. While legal, this is confusing and a linter hazard. Fix: In tools/fs/read.go, rename the variable cap to limit (or similar) to avoid shadowing the builtin cap.
[major] cli/flags.go:77-95 — RegisterStandardFlags does not validate or surface errors from fs.Parse; callers that flag.ContinueOnError and ignore the returned error receive a partially-parsed (and uninspectedly incorrect) StandardConfig. Additionally defaults pointer overrides are inconsistent: MaxTurns: 0 is a valid user intent but is treated as "use default". Fix: In cli/flags.go, change the defaults construction to use an explicit Override struct with *string/*int fields, or document that 0 for MaxTurns means "use default" in the struct comment and add HasMaxTurns bool to StandardConfig.
[major] tools/git/diff.go:86 — DiffBranchTool sets cap := a.MaxBytes and then checks if cap <= 0 before falling back to maxBytes, but does the same check redundantly in ReadFileTool. The variable name cap shadows the builtin. Fix: In tools/git/diff.go, rename cap to limit to avoid shadowing the builtin cap function.

Suggestions

[minor] loop/run.go:122-134 — lastAssistantMsg and lastAssistantNew are tracked purely to tag the last assistant on max-turns exit. This is correct but the mutable index tracking is subtle; a named helper / refactor could simplify the post-loop tagging logic. Fix: In loop/run.go, extract the max-turns tagging into a helper func tagMaxTurns(messages []Message, index int) or just add a clarifying comment before the lastAssistantMsg declarations.
[minor] providers/registry.go:28-34 — Factory has three exported fields with parallel return values in New. Consider collapsing them into a Metadata struct for clarity, or alternatively adding a func (f Factory) KeyAvailable() bool to reduce the external surface area. Fix: In providers/registry.go, add a KeyAvailable() bool method on Factory so providers.KeyAvailable delegates to it, reducing the direct field access pattern in tests.
[minor] tools/fs/blocklist.go:48-49 — The "secrets" entry is a plain directory name and may be overly broad, matching path/to/secrets/config.yaml intended for non-secret usage. Document the trade-off or consider removing it. Fix: In tools/fs/blocklist.go, remove the "secrets" bare directory entry, or add a code comment explaining the rationale for blocking the exact directory name "secrets" at any depth as a generic protection.
[minor] event_helpers.go:26 — WithStreamWriter and WithToolLogger silently drop writer errors, which is documented but may surprise callers in production. Consider adding an io.Writer+error callback variant later. Fix: In event_helpers.go, add a // TODO: consider an error-report variant. comment above WithStreamWriter and WithToolLogger so the README caveat about dropped errors is codified in the source.
[minor] session.go:205-207 — The emit callback is called before config.emit, then config.emit, then the auxEmits in sequence. If config.emit panics, auxEmits stops running — an ordering dependency. Fix: In session.go, wrap the config.emit and for _, aux := range config.auxEmits { ... } into a single helper so the dispatch order is centralized and documented.
[minor] prompts/catalog.go:44-51 — NewCatalog returns an complex interpolated error when defaultVersion is empty. This is fine, but Get("") already defaults to the default version; callers that want to be explicit about the default should not construct with empty. Consider a constant. Fix: In prompts/catalog.go, export a DefaultVersionName = "" constant (or just document in the README that empty string triggers default behavior).

Looks good

The NewTool[T] helper with ErrorResult/TextResult neatly removes ~15 lines per tool definition and provides consistent malformed-argument handling without crashing the loop. The agents/glue-review migration is clean.
StopReasonMaxTurns is precisely the right granularity — the loop's contract remains unchanged, and callers can distinguish budget exhaustion from natural stops and provider truncation without needing to regex the error string.
tools/fs and tools/git are well-scoped as external packages per ADR 0003; the POSIX isolation is maintained and the dependency direction is correct (tools → glue, not vice versa).
Provider registry with self-registration via init() is idiomatic and keeps downstream agent binaries free of hand-coded provider switches.

Open questions

Is tools/fs intended to ever gain write tools, or is it strictly read-only per ADR 0003? The ADR update says "write-side filesystem ... remain out of scope until a follow-up ADR" — should a subsequent ADR define the safety boundary before write tools land, or should the package name reflect the read-only nature?
Should WithFailover support a configurable Context.WithTimeout per provider attempt? A slow first provider could hold up the whole prompt if it hangs inside Stream before emitting the first event.
The Registry lacks an Unregister or Replace mechanism for tests that want to avoid init() side effects. Is registering a "test-fake" in tests the intended pattern, or should there be a package-level test hook?

🤖 Posted by glue-review.

github-actions · 2026-05-07T11:00:30Z

+		ch, err := p.Stream(ctx, req)
+		if err != nil {
+			attempts = append(attempts, FailoverAttempt{Index: i, Err: err})
+			continue


[major] The proxy goroutine in failoverProvider.Stream doesn't handle ctx cancellation when forwarding events from the committed provider. If ctx is canceled after out <- first but before the for ev := range ch loop finishes, the goroutine leaks while blocked reading ch or writing to out (unbounded 16-buffer might full-block, consumers might stop draining)

💡 AI prompt to fix

In `failover.go`, wrap the forwarding goroutine to select on `ctx.Done()` in addition to the `for ev := range ch` loop, and close `out` on context cancellation.

github-actions · 2026-05-07T11:00:30Z

+		}
+
+		// commit: re-emit the first event then forward the rest.
+		out := make(chan loop.ProviderEvent, 16)


[major] return nil, &FailoverError{Attempts: attempts} leaks channels for the successfully Streamed providers that were discarded before successful first-event commit

💡 AI prompt to fix

In `failover.go`, drain the `ch` returned from `p.Stream` in each `continue` fallback path (stream error, empty stream, first-event error), and separately drain the `ch` from every prior iteration of the loop that was never selected, to prevent goroutine leaks from unclosed upstream channels.

github-actions · 2026-05-07T11:00:30Z

+				return glue.ErrorResult(fmt.Errorf("path %q is blocked by sensitive-file pattern %q; do not retry", a.Path, pat)), nil
+			}
+			resolved, err := SafeJoin(workDir, a.Path)
+			if err != nil {


[major] The local variable cap shadows the builtin cap function. While legal, this is confusing and a linter hazard

💡 AI prompt to fix

In `tools/fs/read.go`, rename the variable `cap` to `limit` (or similar) to avoid shadowing the builtin `cap`.

github-actions · 2026-05-07T11:00:30Z

+			d.Model = defaults.Model
+		}
+		if defaults.ID != "" {
+			d.ID = defaults.ID


[major] 95—RegisterStandardFlagsdoes not validate or surface errors fromfs.Parse; callers that flag.ContinueOnErrorand ignore the returnederrorreceive a partially-parsed (and uninspectedly incorrect)StandardConfig. Additionally defaultspointer overrides are inconsistent:MaxTurns: 0` is a valid user intent but is treated as "use default"

💡 AI prompt to fix

In `cli/flags.go`, change the `defaults` construction to use an explicit `Override` struct with `*string`/`*int` fields, or document that `0` for `MaxTurns` means "use default" in the struct comment and add `HasMaxTurns bool` to `StandardConfig`.

github-actions · 2026-05-07T11:00:30Z

+				gitArgs = append(gitArgs, opts.Pathspec...)
+			}
+			out, err := RunGit(ctx, RunOptions{WorkDir: opts.WorkDir, Timeout: opts.Timeout}, gitArgs...)
+			if err != nil {


[major] DiffBranchTool sets cap := a.MaxBytes and then checks if cap <= 0 before falling back to maxBytes, but does the same check redundantly in ReadFileTool. The variable name cap shadows the builtin

💡 AI prompt to fix

In `tools/git/diff.go`, rename `cap` to `limit` to avoid shadowing the builtin `cap` function.

github-actions · 2026-05-07T11:00:30Z

+	// (no explicit API key — falls back to the provider's env var).
+	New func() loop.Provider
+
+	// DefaultModel is the model id callers should use when they have


[minor] 34—Factoryhas three exported fields with parallel return values inNew. Consider collapsing them into a Metadatastruct for clarity, or alternatively adding afunc (f Factory) KeyAvailable() bool` to reduce the external surface area

💡 AI prompt to fix

In `providers/registry.go`, add a `KeyAvailable() bool` method on `Factory` so `providers.KeyAvailable` delegates to it, reducing the direct field access pattern in tests.

github-actions · 2026-05-07T11:00:30Z

+		"credentials",
+		"credentials.json",
+		"service-account*.json",
+		"client-secret*.json",


[minor] 49— The"secrets"entry is a plain directory name and may be overly broad, matchingpath/to/secrets/config.yaml` intended for non-secret usage. Document the trade-off or consider removing it

💡 AI prompt to fix

In `tools/fs/blocklist.go`, remove the `"secrets"` bare directory entry, or add a code comment explaining the rationale for blocking the exact directory name `"secrets"` at any depth as a generic protection.

github-actions · 2026-05-07T11:00:30Z

+		}
+		c.auxEmits = append(c.auxEmits, func(e Event) {
+			if e.Type == EventTextDelta && e.Delta != "" {
+				_, _ = io.WriteString(w, e.Delta)


[minor] WithStreamWriter and WithToolLogger silently drop writer errors, which is documented but may surprise callers in production. Consider adding an io.Writer+error callback variant later

💡 AI prompt to fix

In `event_helpers.go`, add a `// TODO: consider an error-report variant.` comment above `WithStreamWriter` and `WithToolLogger` so the README caveat about dropped errors is codified in the source.

github-actions · 2026-05-07T11:00:30Z

@@ -204,6 +205,9 @@ func (s *Session) Prompt(ctx context.Context, text string, options ...PromptOpti
 			if config.emit != nil {


[minor] 207— Theemitcallback is called beforeconfig.emit, then config.emit, then the auxEmitsin sequence. Ifconfig.emitpanics,auxEmits` stops running — an ordering dependency

💡 AI prompt to fix

In `session.go`, wrap the `config.emit` and `for _, aux := range config.auxEmits { ... }` into a single helper so the dispatch order is centralized and documented.

github-actions · 2026-05-07T11:00:30Z

+		return nil, fmt.Errorf("prompts: read %s: %w", dir, err)
+	}
+	if defaultVersion == "" {
+		return nil, fmt.Errorf("prompts: default version is required (available: %s)", strings.Join(versions, ", "))


[minor] 51—NewCatalogreturns an complex interpolated error whendefaultVersionis empty. This is fine, butGet("")` already defaults to the default version; callers that want to be explicit about the default should not construct with empty. Consider a constant

💡 AI prompt to fix

In `prompts/catalog.go`, export a `DefaultVersionName = ""` constant (or just document in the README that empty string triggers default behavior).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Promote the path-safety / git-shell-out helpers that agents/glue-review inlined into shared, audited extension packages. ADR 0003 was approved months ago but no implementation landed; the agent's tools.go and blocklist.go are empirical proof of the API shape, so port them. tools/fs: - SafeJoin(base, rel) — traversal + symlink defense - Truncate(s, max) — output cap with marker - Blocklist (Default/Merge/Match) — three-way pattern match (whole-path, basename, components, case-insensitive) - ReadFileTool(opts) — ready-to-register read tool composed of the above tools/git: - RunGit(ctx, opts, args...) — PATH lookup, configurable timeout (default 15s), stderr captured into the error - BuildPathspec(includes, excludes) — Git pathspec construction with the catch-all-include rule - DiffBranchTool(opts) — git_diff_branch tool - LogBranchTool(opts) — git_log_branch tool Both packages live outside the core glue package per ADR 0003 so the harness stays free of POSIX coupling. They depend on glue (for Tool / ToolSpec / ToolResult and the new NewTool[T] helper from #88); the core does not depend on them. ADR 0003 marked Implemented; design.md package boundaries updated. Migrating agents/glue-review to use these packages is filed as a follow-up so this PR stays single-purpose and reviewable. Verification: go build ./... # ok go vet ./... # ok go test ./... # ok go test ./tools/fs ./tools/git # ok (table tests + scratch-repo end-to-end) Closes #89 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Stop forcing agents to hand-code provider failover. Today an agent that wants nvidia → openrouter → gemini failover maintains a private map of provider → env var, hand-codes per-attempt session ids to avoid transcript poisoning, and buffers streamed text per attempt. Move the boring parts into the framework. New driver-style registry at providers/registry.go: - providers.Register(name, Factory) — providers self-register in init() - providers.New(name) → (Provider, defaultModel, envKey, error) - providers.Lookup(name), providers.Known() (sorted), providers.KeyAvailable(name) - Case-insensitive lookup; unknown name returns an error listing the registered names Each shipped provider exposes EnvKey + DefaultModel constants and self-registers from init() (gemini, nvidia, openrouter). Importing `_ "github.com/erain/glue/providers/<name>"` makes the name resolvable. WithFailover wrapper in package glue: - glue.WithFailover(provs ...Provider) Provider - Tries each provider in order; falls through on Stream error, immediate ProviderEventError, or empty stream - Commits to a provider once any non-error event is observed — preserves the loop's "no half-streamed transcripts on retry" invariant, no mid-stream recovery - All-providers-failed surfaces as *FailoverError with per-provider attempts (errors.As compatible) README: new "Provider failover" subsection. design.md: provider registry listed under package boundaries. Migrating agents/glue-review off its hand-coded failover is filed as a follow-up so this PR stays single-purpose. Verification: go build ./... # ok go vet ./... # ok go test . ./loop ./providers/... ./tools/... # ok (all green) go test ./agents/glue-review # one live test # rate-limited # by NVIDIA (HTTP 429), # not a regression Closes #90 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Replace the canonical "switch e.Type { case EventTextDelta: ... }" block that every CLI agent writes with two one-line options. - glue.WithStreamWriter(io.Writer) mirrors EventTextDelta.Delta. - glue.WithToolLogger(io.Writer) writes "[tool] <name>\n" on EventToolStart. - Both nil-safe (nil writer = no-op so callers can pass conditional writers without branching) and silently drop writer errors — documented as convenience options, not delivery-guaranteed pipes. - Compose additively with WithEvents and each other via a new promptConfig.auxEmits slice — installing one does not displace any other handler. WithEvents continues to use the existing single-emit field with last-wins semantics. README "Streaming events" subsection rewritten to lead with the helpers; the WithEvents path is kept as the richer-formatting fallback. Verification: go build ./... # ok go vet ./... # ok go test . ./loop ./providers/... ./tools/... # ok (all green) go test . -run "Stream|ToolLogger|StreamHelpers" # ok Closes #91 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Promote agents/glue-review/prompt.go into a small reusable framework helper. Versioning prompts behind an embed.FS is a recurring pattern (A/B test prompts, roll back without rebuild) and the agent's implementation already encodes the right error semantics — unknown version → list available, never silent fallback. New package github.com/erain/glue/prompts: - NewCatalog(fsys fs.FS, dir, defaultVersion) (*Catalog, error) validates that defaultVersion exists at construction time so misconfiguration fails fast. - Get(version) (string, error) — empty version uses the default; unknown returns an error listing every available version. - Versions() (sorted), Default() - Nil-safe getters (return nil/"" / typed error) Returned bodies are right-trimmed of newlines and re-suffixed with a single newline so callers can append further instructions without double-blank-line drift — matches the agent's existing format. Templating / variable substitution remain out of scope: the catalog returns raw bytes; rendering is the caller's job. README: new "Versioned prompts" subsection. Migration of agents/glue-review/prompt.go onto this catalog filed as a follow-up. Verification: go build ./... # ok go vet ./... # ok go test ./prompts -count=1 # ok (8/8) Closes #92 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Today loop.Run errors on max-turns and the partial transcript's last assistant message carries StopReasonToolUse — indistinguishable from a turn that legitimately ended on tools. An agent that wants to retry with a higher budget can't tell "we ran out of turns" apart from "model errored mid-stream". Add StopReasonMaxTurns. When the loop exits via the budget guard, mark the last assistant message in both Messages and NewMessages with this stop reason before returning the (still-non-nil) error. The transcript shape is unchanged; only the StopReason field on one message gains a new value. - New const loop.StopReasonMaxTurns ("max_turns"), re-exported as glue.StopReasonMaxTurns - loop.Run tracks the index of the last appended assistant message and tags it on max-turns exit - design.md "Agent Loop" section documents the new outcome Verification: go build ./... # ok go vet ./... # ok go test ./loop -count=1 -run "MaxTurns|NaturalStop" # ok go test . ./loop ./prompts ./providers/... ./tools/... # ok Closes #93 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Stop having every multi-provider Glue agent re-register the same six flags. New thin package github.com/erain/glue/cli wires --provider, --model, --id, --store, --work, --max-turns onto a caller-supplied flag.FlagSet and returns a typed getter. - cli.StandardConfig — struct of the six common values - cli.StandardFlagDefaults — exported defaults so callers can override one or two without re-registering everything - cli.RegisterStandardFlags(fs, defaults) func() StandardConfig Help text mentions failover semantics for --provider so agents do not need to redocument them. The package is intentionally thin; cmd/glue remains the canonical CLI runner — this is shared infrastructure for downstream agent binaries. README "CLI" section gains a "Standard flags for downstream agents" subsection with usage example. Verification: go build ./... # ok go vet ./... # ok go test ./cli -count=1 # ok (3/3 — defaults, argv parse, defaults override) Closes #94 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions Bot reviewed May 7, 2026

View reviewed changes

erain mentioned this pull request May 7, 2026

docs: refresh README, design.md, project-plan.md for the 0.x surface #102

Merged

2 tasks

erain and others added 8 commits May 7, 2026 10:13

test: replace handrolled intStr with strconv.Itoa

84a5af6

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

tools/{fs,git}: rename cap variable to limit (avoid builtin shadowing)

3422565

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

erain force-pushed the issue/94-cli-flags branch from b2f5eb4 to 2b45fc1 Compare May 7, 2026 14:16

erain merged commit 383f517 into main May 7, 2026
3 of 4 checks passed

erain deleted the issue/94-cli-flags branch May 7, 2026 14:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: StandardFlags helper for downstream agents (closes #94)#101

cli: StandardFlags helper for downstream agents (closes #94)#101
erain merged 9 commits into
mainfrom
issue/94-cli-flags

erain commented May 7, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot May 7, 2026

Uh oh!

github-actions Bot May 7, 2026

Uh oh!

github-actions Bot May 7, 2026

Uh oh!

github-actions Bot May 7, 2026

Uh oh!

github-actions Bot May 7, 2026

Uh oh!

github-actions Bot May 7, 2026

Uh oh!

github-actions Bot May 7, 2026

Uh oh!

github-actions Bot May 7, 2026

Uh oh!

github-actions Bot May 7, 2026

Uh oh!

github-actions Bot May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		@@ -204,6 +205,9 @@ func (s *Session) Prompt(ctx context.Context, text string, options ...PromptOpti
		if config.emit != nil {

Conversation

erain commented May 7, 2026

Summary

Test plan

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Summary

Issues

Suggestions

Looks good

Open questions

Uh oh!

github-actions Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant