feat: profile inheritance via extends field#203
feat: profile inheritance via extends field#203josephgimenez wants to merge 5 commits intoalways-further:mainfrom
extends field#203Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the profile management system by introducing an inheritance mechanism. Users can now define profiles that extend existing ones, inheriting their settings and only specifying additions or overrides. This change aims to streamline profile configuration, minimize redundancy, and prevent configuration drift, all while incorporating robust safety checks to ensure stable and predictable behavior. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature for profile inheritance using an extends field, which will significantly improve the maintainability and reduce duplication in user profiles. The implementation is robust, including safeguards against circular dependencies and excessive inheritance depth. The merge logic is well-thought-out, and the new functionality is accompanied by a comprehensive set of tests. I have identified a minor performance optimization opportunity and a bug in one of the new tests that I've provided suggestions for. Overall, this is a well-executed feature.
dbaa6da to
8410398
Compare
Add a dedicated error type for profile inheritance failures (circular dependencies, depth limits, missing base profiles) to support the upcoming `extends` field in user profiles. Signed-off-by: Joseph Gimenez <joseph.gimenez@joingotu.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
User profiles can now declare `"extends": "base-profile-name"` to inherit filesystem paths, security groups, network settings, and all other fields from a base profile. The child only needs to declare its additions or overrides, reducing duplication and preventing drift when base profiles are updated. Merge strategy: - Vec fields (filesystem paths, groups): append + dedup - HashMap fields (credentials, hooks): child wins on key conflict - Boolean fields (network.block, interactive): OR semantics - Scalar fields (network_profile, workdir): child overrides Safety limits: max 10-level inheritance depth, circular dependency detection via visited-set tracking. Closes always-further#164 Signed-off-by: Joseph Gimenez <joseph.gimenez@joingotu.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
23 new tests covering: - Merge logic: filesystem paths, security groups, deduplication, meta replacement, custom credentials, network profile override, network block inheritance, workdir inherit/override, env credentials child-wins, interactive OR semantics, extends consumed after merge - Loading pipeline: extends built-in profile, extends user profile, three-level chain, missing base error, circular dependency detection, self-reference detection, depth limit enforcement, empty child inherits all base fields - Helpers: dedup_append order preservation, empty vec handling - Deserialization: extends field present and absent Signed-off-by: Joseph Gimenez <joseph.gimenez@joingotu.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test_extends_user_profile test used `&format!(...)` with no interpolated variables. Replace with a plain string literal. Signed-off-by: Joseph Gimenez <joseph.gimenez@joingotu.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Optimize dedup_append to store references in HashSet (one clone per item instead of two) - Rewrite test_extends_user_profile to test actual user-to-user inheritance instead of extending a built-in - Document WorkdirAccess::None dual-purpose limitation in merge logic - Add tests for hooks merge, custom_credentials collision, and trust_groups additive semantics - Add profile inheritance section to CLI README and CHANGELOG entry Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Joseph Gimenez <joseph.gimenez@joingotu.com>
8410398 to
1227d78
Compare
Summary
extendsfield to user profiles for single-inheritance from built-in or user-defined base profilesCloses #164
Usage Examples
Extend a built-in profile with credential injection
Instead of duplicating all of openclaw's paths and settings:
{ "extends": "openclaw", "meta": { "name": "openclaw-creds" }, "network": { "proxy_credentials": ["telegram", "gemini"], "custom_credentials": { "telegram": { "upstream": "https://api.telegram.org", "credential_key": "telegram_bot_token", "inject_mode": "url_path", "path_pattern": "/bot{}/" } } } }This inherits openclaw's filesystem paths, workdir config, security groups, and everything else — the child only adds what's new.
Add extra paths to claude-code
{ "extends": "claude-code", "meta": { "name": "claude-code-extended" }, "filesystem": { "allow": ["/opt/my-tools"], "read": ["/etc/my-app"] } }Paths are appended to the base (not replaced), so all of claude-code's original paths remain.
Chain user profiles
~/.config/nono/profiles/team-base.json:{ "meta": { "name": "team-base" }, "filesystem": { "read": ["/shared/configs"] }, "network": { "block": true } }~/.config/nono/profiles/my-dev.json:{ "extends": "team-base", "meta": { "name": "my-dev" }, "filesystem": { "allow": ["$HOME/projects"] } }my-devinherits team-base's/shared/configsread access andblock: true, then adds its own write path.Merge strategy
metasecurity.groupssecurity.trust_groupsfilesystem.*(6 fields)network.blockbase || childnetwork.network_profilenetwork.proxy_allownetwork.proxy_credentialsnetwork.custom_credentialsenv_credentialsworkdirNone= "not specified" (see note)hooksrollback.*interactivebase || childReview remediation
Addressed findings from 20 AI review agents (10 Gemini 3.1, 10 Codex 5.3):
dedup_appendoptimization — store&Stringreferences in theHashSetinstead of cloning, reducing to one clone per unique item (Gemini inline suggestion)test_extends_user_profilerewrite — original test wrote a base file but the child extendedclaude-code(built-in), never exercising user-to-user inheritance. Rewritten to parse two temp files and merge them directly, avoidingXDG_CONFIG_HOMEraces under parallel test executiontest_merge_profiles_merges_hookscovering independent hooks and same-key collision (child wins)test_merge_profiles_custom_credentials_child_wins_on_collisionverifying child's upstream wins on same-keytest_merge_profiles_trust_groups_additiveconfirming both base and child trust_groups appear in merged resultDeferred to follow-up
WorkdirAccess::Noneambiguity (requires public API change to enum + all consumers)load_embedded_policy()caching (OnceLock)write,allow_file,write_file)deny_unknown_fieldsonProfile/ProfileDefstructsTest plan
-D warnings -D clippy::unwrap_used)🤖 Generated with Claude Code