Conversation
…ras extension schema Port the md2gemini and md2codex reference extensions from Python to Node.js for broader portability (Node ships uniformly across Windows, macOS, and Linux). Add the extension field to extraTargetConfig in the global and project JSON schemas so editors accept it, with a schema test guarding its presence.
…ng, warn on scope change - writeInitGitignore now appends config.yaml to a pre-existing root .gitignore if missing (enforces the safety guarantee instead of best-effort) - single source of truth config.ScopeDir; EffectiveGitRoot/scopeDir/gitRootScopes delegate - warn when --git-root is passed with --remote on an already-initialized repo - use 0o644 and check WriteFile errors; add tests for the append + agents scope
… switch scope - extract shared git.InitScopeRepo/WriteScopeGitignore/EnsureLocalIdentity (UI-free) - cmd init delegates to the shared helpers (no logic duplication) - handler_git operations use EffectiveGitRoot; status exposes the scope - new POST /api/git/root inits a repo at the target scope dir + persists git_root
Add Config.GitRootMismatch() as the single source of truth for detecting when the git repo lives at a different scope than git_root configures, and reuse it across the CLI guidance, the server status response (scopeMismatch /mismatchScope/mismatchDir), and a new warning banner on the Git Sync page. Document git_root in the config structure-view field docs (fieldDocs) and the GitStatus client type, and add the gitSync.scope.* / gitSync.mismatch.* i18n keys across all locales to keep dictionary parity. Cover the behavior with config, server handler, and CLI integration tests: push/pull honor the root scope (agents files reach the remote, config.yaml stays untracked), POST /git/root inits+persists the scope and rejects invalid scopes and project mode, and git status reports scope mismatches.
The md2codex reference converter now carries the agent's `model` frontmatter into the generated TOML, matching what Codex agents expect. Document the extras + extension recipe for converting markdown agents to Codex TOML so the conversion works without a built-in agents target.
Surface the per-target extension transform in the dashboard. A new GET /api/extras/extensions endpoint lists extensions available in the current mode's directory (global or project), and the Extras page renders a picker per target. Selecting an extension forces copy mode (transforms are copy-only) and locks the mode control; changes apply optimistically via the query cache. Adds i18n keys across all locales.
Add an "Extensions" tab to the Config page for managing transform extensions in the active mode (global or project): - GET /api/extensions lists installed extensions with their manifest descriptions, merged with the built-in catalog so users can see what is available to download. - POST /api/extensions/install downloads a whitelisted built-in extension into the active mode's extensions directory. - POST /api/extensions/open opens that directory in the user's editor, mirroring the skill "open in editor" affordance. ExtensionSpec now carries the manifest description so the list can surface it. The UI shows installed extensions with a built-in badge, available built-ins with a Download button, and an Open-directory action. Strings are translated across all 11 locales.
- Fix the `extras list` plain-text example to match current output: real header/separator, `✓` icons, no trailing "synced" word on synced rows, and an extension target labeled `extension: <name>`. - Note the per-target `extension` field in `--json` output. - Document the transform overwrite-safety contract (symlink auto-replace; regular file/directory skipped without --force; --force replaces a conflicting directory wholesale). - Add a one-line pointer to transform extensions in the built-in skill.
Adds per-target transform extensions for extras (pipe each source file through an external script, rename per output_ext), a built-in extension catalog (codex-agents, gemini-commands), CLI + Web UI extension management, and the --force/conflict safety contract for generated output.
… commit Stripped one-shot containers (read-only HOME, unwritable identity, or owner mismatch) can make git init/commit fail during test setup, surfacing as a misleading assertion failure deep inside a test. Add a requireWorkingGit probe that init/config/add/commits in a throwaway temp dir up front and t.Skips with the underlying git error when the environment cannot create commits. The probe touches nothing outside its temp dir and does not change git_root behavior.
Port 3000 collides with other local services, blocking the devcontainer from starting. Move the Docusaurus docs server to 8888 across the docs script, dev-servers orchestration, compose port mapping, and forwarded ports/labels.
Large skill lists made it hard to spot which skills are disabled via .skillignore. Add a status filter to `skillshare list`: press `s` to cycle the All -> Enabled -> Disabled view, shown as a `Status:` chip beside the tab bar. The `s:enabled` / `s:disabled` filter token is also supported for combining status with other tags (e.g. `t:tracked s:disabled`). Toggling a skill's state while a status filter is active re-applies the filter so it leaves the view. Closes #172
Mirror the list TUI status filter in the web dashboard. A single monochrome toggle button sits beside the source filter chips (separated by a divider) and cycles All -> Enabled -> Disabled. Counts are scoped to the active tab and compose orthogonally with the source filter.
…g mismatch guidance The mismatch guidance told users to re-run 'skillshare init', but init warns and ignores --git-root once a repo already exists, so it never relocates the repo. Reword the CLI message to state the real options: run 'git init' at the new scope dir for a fresh history, or 'mv <old>/.git <new>/.git' to keep history, or set git_root to match the existing repo. Add a 'Changing the scope after init' section to the git_root config reference and point the commit/push/pull pages at it, replacing the same misleading 're-run init' phrasing.
Previously 'skillshare init --git-root <scope>' on an already-initialized setup either errored with 'already initialized' (no --remote) or warned and ignored the flag (with --remote), so there was no non-interactive way to change the scope after init — only the web UI or a manual config edit. handleExistingInit now treats 'init --git-root <scope>' (without --remote) as a headless scope switch via switchGitRootScope: it inits a repo at the new scope directory (reusing one already there, sharing doGitInitIfAbsent with the fresh-init and server paths) and persists git_root. It does not relocate an existing repo — switching scope means 'version a different directory', not 'move history'. Combined with --remote the flag still falls through to the remote path, now pointing users at the standalone form instead of a bare 'ignored' warning. Update the mismatch guidance to offer 'skillshare init --git-root <scope>' as the first fix, and revise the git_root docs (configuration + init) to document the headless switch, superseding the earlier 'must re-run init / not supported' wording. Add TestGitRoot_SwitchScopeAfterInit.
Project init rejected --git-root (and other git flags) with a bare 'unknown option', which is confusing when the user just forgot that git integration is global-only — easy to hit when the current directory has a .skillshare/ and init auto-detects project mode. Match git-related flags (--git-root, --git, --no-git, --remote) explicitly and explain they are global-only, pointing at 'skillshare init -g <flag>'. Add TestInitProject_GitRootFlag_RejectedAsGlobalOnly.
Combining --git-root <scope> with --remote <url> on an already-initialized setup now switches the scope and wires the remote onto the new scope's repo in one command, instead of warning and ignoring --git-root. Reuses the existing scope-switch and remote-configuration paths back to back.
The Git Sync scope switcher now accepts an optional remote URL, reaching UI parity with the CLI's `init --git-root <scope> --remote <url>`. The POST /api/git/root handler takes an optional remoteURL and wires origin on the just-initialized scope repo via a new git.SetOrAddRemote helper (adds origin when absent, updates the URL otherwise). Adds gitSync.scope.remote* strings across all locales.
…emote A remote URL beginning with '-' could be parsed by git as an option flag (argv injection, e.g. --upload-pack=...). Validate the URL (reject a leading dash) and pass '--' before positional args so git stops option parsing. Covers both the new scope-switch remote path and the existing skill-repo URL editor that funnels through SetRemoteURL.
The CLI addRemote ran a raw "git remote add origin <url>" with the user-supplied URL, sharing the argv flag-smuggling risk just fixed in the git package. Delegate to git.SetOrAddRemote so the --remote flag goes through the same leading-dash rejection and "--" terminator, removing the duplicated exec path.
The not-a-repo state was a dead end: the scope switcher only rendered once a repo existed, so there was no way to initialize git or set a remote from the UI. Add an 'Initialize git repository' button that opens the existing scope dialog (with optional remote) — POST /api/git/root already inits the scope repo and wires origin. Dialog title/message adapt for the init case. Also detect a missing git executable: gitStatus reports gitInstalled (via exec.LookPath), the page shows a clear 'Git is not installed' notice instead of a cryptic 500 on init, and handleSetGitRoot rejects with a plain message.
- Move the git_root scope selector from its own row into the status bar alongside branch/HEAD, matching the repo-info layout - Lay out the not-a-repo Initialize button inline (justify-between) so it no longer floats awkwardly below the warning row
…eaks At the root git_root scope, `git add -A` over the config base directory records any subdirectory that has its own .git (e.g. the skills source) as an empty submodule, and can commit the machine-specific config.yaml. - Detect nested repos and warn before commit/push (CLI) or surface them on the Git Sync page with a one-click "Disable nested .git" action (UI), which renames .git to .git.disabled (reversible) and drops any stale gitlink from the index so the directory's files track as normal blobs. - Keep config.yaml out of a root-scope repo automatically: ensure it is ignored and untrack it (git rm --cached) before staging, on both the CLI and the web UI commit/push paths. - Expose nestedRepos/configTracked in GET /api/git/status and add POST /api/git/absorb-nested (validated against detected nested repos).
An unknown git_root value silently fell back to the skills scope via ScopeDir's default case, so a typo like "agnets" made commit/push/pull operate on the wrong repository without any warning. Validate the scope at the commit/push/pull entry point (resolveGitRoot) and in ValidateConfig, reusing the existing ValidGitRoot check backed by a new ValidGitRoots list.
…rver The dashboard computed extension target status with EffectiveMode, which defaults an empty mode to merge, so a valid transform extension target (extension set, no explicit mode) was always reported as drift. Introduce sync.ResolveExtensionMode as the single source of truth (empty/copy resolve to copy, anything else errors) and use it across the CLI, sync, status, and diff paths.
A transform-mode dry-run counted every file as synced without inspecting the target, so it claimed it would sync files that a real non-force sync would skip. Report an existing real file (under the transformed name) as a skip needing --force, mirroring the non-dry-run path. The dry-run still never spawns the extension subprocess, so content is compared conservatively by existence rather than by output.
The grouped (folder tree) view ignored statusFilter in its isSearching, clear-filters, and empty-state logic, so filtering to Enabled/Disabled with no matches looked like an empty library instead of a filtered view with no results. Mirror the grid/table behavior by including statusFilter in all three checks.
Root-scope git operations detected hazards but proceeded anyway, letting preventable mistakes reach the remote: - Nested git repos were only warned about, then committed/pushed as empty submodules (silent data loss). Now commit/push abort (CLI non-zero exit, server 400) until the nested repos are disabled, and the web UI disables the commit/push buttons while any are present. - The pre-stage safety sweep ran before the dry-run check, so `--dry-run` wrote .gitignore and ran `git rm --cached config.yaml`. The sweep/guard is now strictly read-only on dry-run and only reports what would change. - Server git endpoints used EffectiveGitRoot() without validating git_root, so a hand-edited invalid scope silently fell back to skills. They now share a gitSource() guard that rejects invalid scopes with 400, mirroring the CLI's resolveGitRoot.
The /api/extras/diff path resolved the copy mode but ignored a transform extension's output_ext, so buildExtrasDiffItems compared source relative paths against the target directory. After a .md -> .toml transform sync, the diff looked for the original .md name and reported false drift, even though the status path already handled this. Export ApplyOutputExt as the single source of truth for source->target filename mapping (previously unexported applyOutputExt) and use it in the diff path so sync, status, and diff all agree on the transformed name.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new git_root configuration scope to control which directory is versioned, adds support for "extension transforms" to convert Markdown extras into tool-specific formats (such as TOML for Gemini and Codex) during sync, and implements enabled/disabled status filtering for skills. The review feedback highlights several key areas for improvement, including optimizing concurrency by avoiding holding write locks during slow external git operations, adding timeouts to external command executions, using JSON.stringify for robust TOML string escaping, quoting paths in user-facing CLI guidance to handle spaces, and properly handling or logging ignored errors from extension resolution.
| s.mu.Lock() | ||
| defer s.mu.Unlock() | ||
|
|
||
| dir := config.ScopeDir(s.cfg, body.Scope) | ||
| if _, err := git.InitScopeRepo(dir, body.Scope); err != nil { | ||
| writeError(w, http.StatusInternalServerError, "failed to initialize git at scope: "+err.Error()) |
There was a problem hiding this comment.
Holding the write lock s.mu.Lock() while executing slow external commands in git.InitScopeRepo blocks the entire server (including read requests) during disk I/O and process spawning. Consider acquiring a read lock s.mu.RLock() to resolve the directory, releasing it before calling git.InitScopeRepo, and only acquiring the write lock s.mu.Lock() when updating and saving the configuration.
| s.mu.Lock() | ||
| defer s.mu.Unlock() |
There was a problem hiding this comment.
The handler acquires a write lock s.mu.Lock() but only performs read operations on s.cfg (and executes slow external git commands). This unnecessarily blocks the entire server. Since no configuration changes are written, you should use a read lock s.mu.RLock() to read the required paths, release it immediately, and run the git operations without holding any lock.
| cmd := exec.Command(spec.Run[0], spec.Run[1:]...) | ||
| cmd.Dir = spec.Dir | ||
| cmd.Stdin = src | ||
| cmd.Env = os.Environ() | ||
| for k, v := range env { | ||
| cmd.Env = append(cmd.Env, k+"="+v) | ||
| } | ||
| var stdout, stderr bytes.Buffer | ||
| cmd.Stdout = &stdout | ||
| cmd.Stderr = &stderr | ||
| if err := cmd.Run(); err != nil { |
There was a problem hiding this comment.
The external command is executed using exec.Command without any timeout or context. If an extension hangs (e.g., waiting for input or in an infinite loop), it will block the execution indefinitely, potentially leading to resource exhaustion. Consider using exec.CommandContext with a reasonable timeout to ensure robust execution.
| function tomlString(value) { | ||
| return `"${String(value).replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; | ||
| } |
There was a problem hiding this comment.
The custom tomlString implementation only escapes backslashes and double quotes, but does not escape control characters (like \n, \r, \t, etc.), which can result in invalid TOML if the value contains them. A much simpler and 100% robust way to produce a valid TOML basic string is to use JSON.stringify(String(value)).
| function tomlString(value) { | |
| return `"${String(value).replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; | |
| } | |
| function tomlString(value) { | |
| return JSON.stringify(String(value)); | |
| } |
| function tomlString(value) { | ||
| return `"${String(value).replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; | ||
| } |
There was a problem hiding this comment.
The custom tomlString implementation only escapes backslashes and double quotes, but does not escape control characters (like \n, \r, \t, etc.), which can result in invalid TOML if the value contains them. A much simpler and 100% robust way to produce a valid TOML basic string is to use JSON.stringify(String(value)).
| function tomlString(value) { | |
| return `"${String(value).replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; | |
| } | |
| function tomlString(value) { | |
| return JSON.stringify(String(value)); | |
| } |
| ui.Info(" but the git repo lives at: %s (%s)", dir, scope) | ||
| ui.Info(" Fix it with one of:") | ||
| ui.Info(" - skillshare init --git-root %s (start a fresh repo at the configured scope)", configured) | ||
| ui.Info(" - mv %s/.git %s/.git (move the existing repo over, keeps history)", dir, root) |
There was a problem hiding this comment.
If the directory paths contain spaces (which is common, especially on Windows), the suggested mv command will fail when copy-pasted into the shell. Consider wrapping the paths in double quotes to make the command robust.
| ui.Info(" - mv %s/.git %s/.git (move the existing repo over, keeps history)", dir, root) | |
| ui.Info(" - mv \"%s/.git\" \"%s/.git\" (move the existing repo over, keeps history)", dir, root) |
| lines = append(lines, | ||
| "", | ||
| "Their files will NOT be tracked. Disable each nested repo, then retry:", | ||
| fmt.Sprintf(" mv %s/<dir>/.git %s/<dir>/.git.disabled", dir, dir), |
There was a problem hiding this comment.
If the directory paths contain spaces, the suggested mv command will fail when copy-pasted. Consider wrapping the paths in double quotes.
| fmt.Sprintf(" mv %s/<dir>/.git %s/<dir>/.git.disabled", dir, dir), | |
| fmt.Sprintf(" mv \"%s/<dir>/.git\" \"%s/<dir>/.git.disabled\"", dir, dir), |
| if spec, rerr := resolveExtension(t.Extension, extensionsDir); rerr == nil && spec != nil { | ||
| outputExt = spec.OutputExt | ||
| } |
There was a problem hiding this comment.
The error rerr returned by resolveExtension is silently ignored. If an extension is misconfigured (e.g., a typo in extension.yaml or a missing file), the sync status will silently fall back to comparing the original .md files, leading to confusing 'drift' or 'not synced' statuses without any error message. Consider logging a warning or surfacing the error to the user.
| if t.Extension != "" { | ||
| if spec, serr := s.resolveExtensionSpec(t.Extension); serr == nil && spec != nil { | ||
| outputExt = spec.OutputExt | ||
| } | ||
| } |
There was a problem hiding this comment.
The error serr returned by resolveExtensionSpec is silently ignored. If an extension is misconfigured, the sync status will silently fall back to comparing the original .md files, leading to confusing 'drift' or 'not synced' statuses without any error message. Consider logging a warning or surfacing the error to the user.
| if spec, serr := s.resolveExtensionSpec(t.Extension); serr == nil && spec != nil { | ||
| outputExt = spec.OutputExt | ||
| } |
There was a problem hiding this comment.
The error serr returned by resolveExtensionSpec is silently ignored. If an extension is misconfigured, the diff path will silently ignore the output extension, leading to false drift reports. Consider logging a warning or handling the error.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e9b4fb370
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| src, ok := s.gitSource(w) | ||
| if !ok { | ||
| return | ||
| } |
There was a problem hiding this comment.
When git_root is set to root, this src is the config directory, but the post-pull auto-sync below still passes it to DiscoverSourceSkills/SyncTarget* as if it were the skills source. In the dashboard Pull flow, a skill at skills/foo/SKILL.md is discovered with rel path skills/foo, so default flat targets get skills__foo (or copy the wrong subtree) instead of updating foo; git_root=agents/extras similarly points skill sync at the wrong tree. Use the git root only for the git pull and keep syncing from s.cfg.EffectiveSkillsSource() as the CLI path does via skillshare sync --global.
Useful? React with 👍 / 👎.
- handler_git: dashboard Pull auto-sync now uses EffectiveSkillsSource() instead of the git_root scope dir, so it no longer mangles skill rel paths (skills/foo -> skills__foo) when git_root is root/agents/extras - extension: run the transform subprocess via CommandContext with a 30s timeout so a hung extension can't block sync indefinitely - md-toml.js (codex-agents, gemini-commands): escape values via JSON.stringify so control characters produce valid TOML basic strings - gitroot: quote paths in the commit/push mv guidance so paths with spaces can be pasted safely - extras list + extras server handlers: warn when an extension fails to resolve instead of silently falling back and reporting false drift Lock-contention suggestions on handleSetGitRoot/handleAbsorbNested are not applied: this is a single-user localhost dashboard, and dropping the write lock around .git-mutating operations trades safety for negligible throughput.
1 participant
New Features
Git scope control (
git_root)git_rootscope — choose which directoryskillshare commit,push, andpullversion. The default stays your skills source, but you can point git atagents,extras, orroot(skills + agents + extras together in a single repo). Set it during init, or switch later on an existing setup:root-scope repo automatically keepsconfig.yamlout of version control (it holds machine-specific paths), and nested git repositories are detected and blocked before they would upload as empty submodules. Ifgit_rootpoints to a scope whose directory has no repo,commit/push/pullprint a "Git root mismatch" error with the exact commands to fix it.git_rootscope, set the git remote during the switch, and offers a one-click action when the scoped directory isn't a repository yet.Extras extension transforms
extensionfield on extras targets — convert Markdown into a tool's native format during sync, for tools that don't read Markdown. Reference extensions ship for Gemini CLI (TOML commands) and Codex CLI (TOML agents):extras collectskips them), usecopysemantics, and never overwrite a local file or directory without--force. The Codex agents extension mapsname,description, andmodelfrom frontmatter.List filtering
sin thelistTUI to cycle All → Enabled → Disabled, or use thes:enabled/s:disabledtag to combine status with other filters. The dashboard Resources page gains the same status filter.Bug Fixes
-(which git could misinterpret as a flag) are now rejected when setting or adding a remote, including viaskillshare init --remote.