feat(agent-config): hash-guarded read/save, editor UI, JS bundling #76
No reviewers
Labels
No labels
bug
commercial
documentation
duplicate
enhancement
feature
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
jasoncouture/llama-shears!76
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/agent-config-hash"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Test plan
🤖 Generated with Claude Code
Pull request overview
This PR extends LlamaShears’ agent configuration pipeline to support hash-based change detection and optimistic-concurrency saves, and adds a web UI for listing/editing agent JSON configs (with a new JS bundling pipeline). It also includes a broad set of supporting runtime/UI refinements (logging generator refactors, context-compaction heuristics, token timestamping, and eventing refactors).
Changes:
IAgentConfigProvider.ReadFileAsync/SaveAsyncAPIs with backup rotation and optimistic concurrency./agentslisting and/agents/{id}/editeditor UI (CodeMirror) plus an esbuild-based JS bundling MSBuild target and Docker build support.LoggerMessageusages.Reviewed changes
Copilot reviewed 82 out of 90 changed files in this pull request and generated 6 comments.
Show a summary per file
IAgentConfigProviderto satisfy new interface members.InferenceRunnerconstruction to include logger dependency.InferenceRunnerconstruction to include logger dependency.InferenceRunnerconstruction to include logger dependency.InferenceRunnerconstruction to include logger dependency.InferenceRunnerconstruction to include logger dependency.InferenceRunnerconstruction to include logger dependency.InferenceRunnerconstruction to include logger dependency.Hashto loaded configs (non-serialized change token).EventBusinto a named type.EventBusinto its own type./agentslisting page linking to editor routes./agents/{id}/editeditor with hashing/conflict banners and local drafts.dist/site.js.SaveAgentConfigResult.Ok.SaveAgentConfigResult.InvalidJson.SaveAgentConfigResult.Conflict.SaveAgentConfigResult.AgentConfigFile.Hashparameter/property.dist/directories.Files not reviewed (1)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TryJoinContextScope can throw if a different scope is already present on this call chain, and it also leaves IDataContextFactory.Current set for subsequent UI operations. Consider capturing/restoring the prior scope (or calling ClearCurrent in a finally) so one slash command doesn't permanently pin the circuit to an agent scope or crash when the selected agent changes.
@ -65,0 +78,4 @@var hash = Convert.ToHexString(SHA256.HashData(bytes));return new AgentConfigFile(Encoding.UTF8.GetString(bytes), hash);}ReadFileAsync builds the on-disk path from the untrusted agentId via Path.Combine(..., agentId + ".json"). With values like "../foo" this can traverse outside the agents directory, enabling arbitrary file reads when the editor UI hits this API. Consider validating agentId as a safe file name (no directory separators / no ".." segments), or compute the full path and reject anything outside the agents root before accessing the file system.
@ -65,0 +96,4 @@{var existing = await File.ReadAllBytesAsync(path, cancellationToken);currentHash = Convert.ToHexString(SHA256.HashData(existing));}SaveAsync also builds the target path from the untrusted agentId (Path.Combine(..., agentId + ".json")). If agentId contains path traversal sequences, this becomes an arbitrary file write via the editor save endpoint. Please constrain agentId to a safe file name / verify the resulting full path stays under the agents directory before writing, copying backups, or renaming temp files.
@ -65,0 +128,4 @@var tempPath = path + ".tmp";await File.WriteAllBytesAsync(tempPath, bytes, cancellationToken);File.Move(tempPath, path, overwrite: true);Backup filenames use DateTimeOffset.UtcNow.ToUnixTimeSeconds() as the unique suffix. Multiple saves within the same second will attempt to create the same ".{unixSeconds}.bak" name and File.Copy(..., overwrite: false) will throw, causing the save to fail. Use a higher-resolution timestamp (e.g., milliseconds/ticks) and/or add a random suffix / retry loop to guarantee uniqueness.
@ -65,0 +163,4 @@private static long ParseBackupTimestamp(string fullPath){var fileName = Path.GetFileName(fullPath);if (!fileName.EndsWith(".bak", StringComparison.Ordinal))This catch block swallows IOException without any rationale. Per ADR-0010 (docs/adr/0010-exception-handling-requires-justification.md), swallowed exceptions should include an explicit "Ignored — ..." justification (and/or logging) so reviewers/operators understand why deleting stale backups can safely fail here.
LogToolCall records full tool-call arguments at Information level (args={Arguments}). Tool arguments can be large and may include sensitive data (paths, file contents, tokens). Consider lowering this to Debug/Trace and/or redacting/truncating arguments to avoid leaking data into logs and to keep log volume manageable.