fix(source-control): move git out of skill pre-compute so worktree-isolated agents can invoke these skills

[kyle-sexton]

Moves git out of the ## Pre-computed context block in all five git-bearing source-control
skills, so they can be invoked from a worktree-isolated agent. Sibling of #1676 (session-flow).

No linked issue — this is a partial remediation of #1619 (12 of 43 affected skills, together with
#1676), so it must not auto-close it. #1661 tracks the remaining 31 skills across 16 plugins.

The defect

The harness composes an entire pre-compute block into one shell invocation, and the
worktree-isolation Bash guard refuses that composed command when it contains git and is too complex
to statically verify:

This agent is isolated in the worktree …, but this command is too complex to verify that it stays
inside the worktree; break it into plain, separate commands.

So commit, pull-request, worktree, resolve-conflicts, and babysit-prs all failed at
load from an isolated agent. worktree is the sharpest case — the skill for managing worktrees
could not be invoked from inside one.

Mechanism confirmed by direct probe (full write-up on #1619): code-tidying:batch-simplify (one
git pre-compute line, compound) loads; knowledge:course-digest (four complex non-git lines)
loads; session-flow:find-handoff (four lines, one pipe-free git line) is refused. The
trigger is git inside a composed block, not per-line complexity.

The change

Delete the git lines from each pre-compute block; re-acquire those values in the skill body as
individual Bash calls, one command per call. Non-git pre-compute lines are untouched — commit
keeps its exec-bit and user-global config probes, babysit-prs keeps both gh lines.

Three things worth review attention:

1. commit's config-layer probes were themselves compound one-liners that re-derived the
   repository root inline (R="$(git rev-parse --show-toplevel)" && git -C "$R" ls-files …). Moving
   them across intact would have reproduced the defect as a body call, so they are decomposed:
   resolve the root once with git rev-parse --show-toplevel, then substitute the literal path into
   the tracked-layer ls-files check and the personal-overlay test -f. The root-anchoring
   requirement and the present but UNTRACKED rule are unchanged.
2. Failure handling. The old lines carried 2>/dev/null || echo "unknown"; plain Bash calls do
   not reproduce that, so each gather block now says explicitly that a failed command means
   "unknown, carry on" rather than surfacing a raw error.
3. Two reference spokes went stale and are corrected. commit/reference/exec-bit.md no longer
   calls the config-layer probes pre-computed. pull-request/reference/create.md's --pushed
   section justified ignoring the session context partly on the grounds that a !-substituted line
   cannot be git -C-redirected — no longer true once these are ordinary Bash calls. The section
   keeps its instruction (re-resolve explicitly from the target worktree) on the reason that still
   holds (session cwd is the wrong branch for an out-of-tree orchestrator).

shell: bash stays on every skill, including the three that now have no ! lines at all — inert
without pre-compute lines, and removing it would be a frontmatter-contract change with no
behavioral benefit.

babysit-prs is held at exactly 499 lines (net-zero), so this does not consume the single line
it has left under the 500-line hard cap (#1626).

Known pre-existing CI failure — not introduced here

babysit-prs/scripts/engine.test.sh fails on clean main at 9f73fc2e, before any change in
this PR:

FAIL: test_every_refusal_row (test_guards.RefusalsFireOnArgumentShape.test_every_refusal_row)
      (row='merge.wrapper-reaches-failclosed-cli')     AssertionError: 1 != 3
      (row='resolve.wrapper-reaches-failclosed-cli')   AssertionError: 1 != 3
      (row='resolve.wrapper-filters-nothing')          AssertionError: 1 != 2

main stays green because check-changed-skills only runs a skill's script tests when that skill
changes, and babysit-prs had not changed. This PR touches it, so the gate runs and surfaces the
latent failure. Verified pre-existing by running the test on an unmodified main checkout — this PR
changes only SKILL.md prose and touches no engine code. Filed as #1678; not fixed here, since
folding an unrelated guard-test repair into a documentation-shaped migration would obscure both.

Verification

• Proven by probe: four plain git commands (rev-parse --show-toplevel, branch --show-current,
  status --porcelain, log --oneline -5) each succeed as ordinary Bash calls from inside a
  worktree-isolated agent — that is what the replacement bodies rely on.
• Proven by probe: a multi-line pre-compute block containing no git loads fine under isolation.
  Every skill here is now exactly that shape.
• Not directly observed, and deliberately not faked: invoking the edited skills from an
  isolated agent. Skills load from the version-keyed plugin cache
  (~/.claude/plugins/cache/<marketplace>/<plugin>/<version>/), so 0.33.2 does not exist there
  until this merges and plugins are updated. Post-merge confirmation: update plugins, then invoke
  /source-control:worktree from an isolated agent, with knowledge:course-digest as the positive
  control.
• CI cannot prove this fix — it never invokes a skill from an isolated agent.

Local gates against origin/main: check-changelog-parity --check-bump pass,
check-skill-portability pass, markdownlint-cli2 0 issues, check-changed-skills 4 of 5 pass with
the fifth failing only on the pre-existing engine.test.sh above.

Fresh-docs

https://code.claude.com/docs/en/skills fetched this session (2026-07-26): the page documents
"Each !`<command>` executes immediately (before Claude sees anything)" and says nothing about
composing the block into a single invocation — the observed behavior contradicts it. It also
confirms shell: accepts bash (default) or powershell, which is why leaving shell: bash is
inert.

Related

• fix(session-flow): move git out of skill pre-compute so worktree-isolated agents can invoke these skills #1676 — the session-flow half of this remediation.
• fix(skills): worktree-isolated agents cannot invoke a skill whose pre-compute block contains a $-expansion (21 skills; NOT git — see correction comment) #1619 — the defect, the confirmed composition mechanism, and the full 49-skill sweep.
• fix(skills): #1619 remainder — re-scope to the $-expansion trigger (original 31-skill git-based list is WRONG) #1661 — the remaining 31 skills across 16 plugins.
• ci(skills): no gate catches a second precompute line silently breaking a git-bearing skill under worktree isolation #1662 — no gate catches a second pre-compute line silently re-breaking a git-bearing skill.
• babysit-prs engine.test.sh: 3 python-cli refusal rows fail only when a local uv python3 shim leaks PYTHONHOME into the wrappers' py -3 — not a repo bug; passes on Linux CI #1678 — the pre-existing engine.test.sh failure this PR surfaces but does not cause or fix.
• chore(source-control): babysit-prs/SKILL.md also sits 1 line under the 500-line hard cap #1626 — the babysit-prs line-cap headroom this PR deliberately does not consume.

[claude]

Claude finished @kyle-sexton's task in 3m 56s —— View job

---

PR Review

• Gather context and understand the PR
• Review changed skill files
• Review reference doc updates
• Review CHANGELOG and plugin.json version bump
• Assess correctness of decomposed git commands
• Check alignment with CLAUDE.md / project guidelines
• Post final review

---

Overall: This is a sound, well-scoped fix. The mechanism is correctly identified — the harness composes the entire pre-compute block into one shell invocation, and the worktree-isolation guard refuses it when it contains git — and the remedy (individual body-level Bash calls) is the right one. The PR description is exemplary. A few items worth attention:

---

Finding 1 — commit: "present but UNTRACKED" determination is now incomplete

plugins/source-control/skills/commit/SKILL.md L29–31

The original compound one-liner produced one of three distinct outputs:

• present (tracked) — file present and git-tracked → valid config layer
• present but UNTRACKED — not a config layer — file at path but not tracked → explicit user warning
• absent — no file at that path

The decomposed replacement instructs the model to run git -C <root> ls-files --error-unmatch .claude/source-control.md and says:

A file sitting at that path that this command does not report is present but UNTRACKED

But the instruction doesn't tell the model how to determine that the file is "sitting at that path" when ls-files exits non-zero. Without an explicit test -f <root>/.claude/source-control.md step, the model can't distinguish "untracked" from "absent". Practically the config-layer selection is correct either way (neither case is a valid layer), but the ability to warn the user — "your source-control.md is present but not tracked; it won't be applied" — is lost.

The downstream note at L44 still references present but UNTRACKED as a recognized classification, so the intent is preserved but the path to reach it is no longer spelled out.

Suggested addition after step 2:

If ls-files exits non-zero, additionally run test -f <root>/.claude/source-control.md to distinguish present but UNTRACKED (warn the user; do not use as a config layer) from absent.

Fix this →

---

Finding 2 — babysit-prs: prose maintenance note embedded inside ## Pre-computed context

plugins/source-control/skills/babysit-prs/SKILL.md L15

Branch and working tree: gather with two separate Bash calls …
They moved out of pre-compute in #1619 — … so run them individually and do not fold them back.

This line is a prose maintenance note sitting inside a section that the harness normally uses only for !-substituted values. It's harmless (the harness ignores plain text there), but:

1. A reader skimming the pre-compute block is likely to miss that this isn't a computed value.
2. Future editors may not recognize the "do not fold them back" rationale without the surrounding context.

The other four skills moved the gather instruction into a dedicated ## Repository context — gather first section, which is cleaner and consistent. Since babysit-prs still has two real ! lines that must stay in ## Pre-computed context, a simple option is to add a separate ## Repository context — gather first section immediately after the pre-compute block, matching the sibling skills. The 499-line cap makes this tight, but the existing note is already two of those lines, so this is neutral on line count.

---

Observation — removed output-truncation pipes (no action needed)

Several pre-compute commands carried truncation pipes that the new gather instructions omit:

Skill	Old	New
commit	git diff --cached --stat | tail -1	git diff --cached --stat
commit	git status --short | head -20	git status --short
resolve-conflicts	git status | head -4	git status
worktree	git worktree list | head -30	git worktree list

These pipes were appropriate in pre-compute (context is injected as literal text up front). In the body, the model runs the command as a tool call and can handle richer output — the full stat, all conflicted paths — which is actually more useful. A single-pipeline call like git status --short | head -20 is also not a compound command, so it would not trip the worktree guard if the model includes the pipe itself. Not a defect; just noting it's a deliberate departure from the original truncation behavior.

---

Everything else looks correct

• Version bump (0.33.1 → 0.33.2): Appropriate for a fix that changes consumer-visible behavior (skill load from isolated agents).
• shell: bash retained: Confirmed inert without ! lines; keeping it avoids a frontmatter-contract change.
• commit's test -f "$HOME/.claude/source-control.md" stays in pre-compute: Correct — it has no git dependency and composes safely.
• exec-bit-check.sh --probe stays in pre-compute: Correct — it anchors itself via git rev-parse internally and is a single command the harness can verify.
• babysit-prs net-zero on lines: Confirmed — exactly 499 lines.
• pull-request/reference/create.md update: The revised rationale is accurate. The original claim ("a !-substituted line cannot be git -C-redirected") is no longer true; the new text correctly preserves the instruction while correcting the stated reason.
• commit/reference/exec-bit.md: The one-word removal ("pre-computed") is accurate and precise.
• worktree/SKILL.md motivation note: "which made the worktree skill itself uninvocable from inside a worktree" is the clearest articulation of the defect's irony — good to have it here.
• CHANGELOG entry: Thorough, accurate, correctly scoped.

[claude]

Claude finished @kyle-sexton's task in 2m 3s —— View job

---

Security Review — PR #1679

• Read PR diff and changed files
• Analyze shell command injection surfaces
• Check for path traversal / unsafe variable expansion
• Review credential / secret handling
• Assess trust-boundary and authorization gaps
• Post findings

---

No exploitable security vulnerabilities found.

All nine changed files are markdown skill-instruction files and documentation. No executable scripts, GitHub Actions workflows, credential-handling code, or harness engine code is touched. The analysis below covers the surfaces that warranted explicit inspection.

Path substitution in commit/SKILL.md

The instructions at commit/SKILL.md L27–32 direct the model to run git rev-parse --show-toplevel and substitute the result into two subsequent commands (git -C <root> ls-files … and test -f <root>/…). The substitution is not mandated to be quoted.

• Injection risk: effectively zero. Git normalizes the show-toplevel output to a canonical absolute filesystem path; it cannot contain shell metacharacters such as ;, `, or $(). No attacker-controlled value flows into the path — the repository root is set by whoever created the worktree, not by repository content.
• Correctness risk: a repository path that contains spaces would cause an unquoted substitution to break the command. Per the skill's "treat failure as unknown and carry on" instruction, this silently degrades to "layer absent" rather than producing an exploitable state. Not a security concern.

Worktree isolation guard — decomposition

The PR description is explicit that decomposing the compound pre-compute block into individual body calls is what allows the guard to pass each command. All decomposed commands are read-only git introspection (branch --show-current, status --porcelain, log, diff --cached --stat, rev-parse, ls-files, worktree list). None write to the repository or the filesystem. The decomposition does not lower the effective security bar — these commands were safe before, and individually they remain so.

Remaining pre-compute lines (commit, babysit-prs)

The two lines that stay in the pre-compute block are unchanged from main:

• bash "${CLAUDE_PLUGIN_ROOT}/skills/commit/scripts/exec-bit-check.sh" --probe — CLAUDE_PLUGIN_ROOT is a harness-set environment variable, not user-controlled. The script path is fixed.
• gh api user --jq .login and gh pr list --state open --author "@me" --limit 200 … — read-only GitHub API calls scoped to the authenticated user. No secret, token, or credential is echoed into the skill context.

Loss of 2>/dev/null suppression

Old pre-compute lines silently discarded stderr. The new body instructions say "treat a failure as an unknown value and carry on." Git error output on these read-only commands contains no tokens, secrets, or credentials — the worst case is a path or branch name appearing in an error message visible to the model. Not a meaningful security surface.

git worktree list output no longer bounded

The old pre-compute line piped through head -30; the new instruction does not. A repository with an abnormally large number of worktrees would produce more output. This is a resource-consumption note, not an exploitable vulnerability.

---

Verdict: The PR is a documentation-layer migration with no security regressions. The decomposed commands are all read-only, the substitution surface carries no attacker-reachable values, and no credentials or sensitive data move across trust boundaries.

[claude]

Claude finished @kyle-sexton's task in 1m 52s —— View job

---

PR Review (sync — post 7cd4f1b)

• Gather context on the updated PR
• Review the new commit changes
• Assess whether prior review findings were addressed
• Post final review

---

What changed in the sync commit

7cd4f1b (fix(source-control): restore the dropped output bounds and state the verification precisely) proactively addresses the output-bounds observation noted in the sibling PR review before it was formally reported here. The solution is well-reasoned: instead of restoring the original pipes (git status --short | head -20, git diff --cached --stat | tail -1, git worktree list | head -30, git status | head -4), which would be compound commands and reintroduce the defect, each bound is restated as a reading rule — the model is instructed to honor the bound when processing the tool call result. This is the correct approach given the constraint.

The CHANGELOG is also sharpened: it now states precisely what was and was not verified (unfixed skills refused, plain body calls succeed, edited skills not yet exercised from an isolated agent), which is more accurate than the prior wording.

---

Finding 1 — commit: "present but UNTRACKED" is still unreachable (OPEN from first review)

commit/SKILL.md L33–35

The second commit did not address this. The text at L33–35 reads:

A file sitting at that path that this command does not report is present but UNTRACKED, and is deliberately not a config layer.

But without an explicit test -f <root>/.claude/source-control.md step after ls-files exits non-zero, the model cannot determine whether the file is present but untracked or simply absent. Both cases produce the same observable — a non-zero exit from ls-files. The downstream note at L48 still refers to present but UNTRACKED as a recognized classification that produces a user warning, so the intent is intact but the path to reach it is missing.

Suggested addition between step 2 and step 3 of the gather block:

If ls-files exits non-zero, additionally run test -f <root>/.claude/source-control.md to distinguish present but UNTRACKED (warn the user; do not use as a config layer) from absent.

Fix this →

---

Finding 2 — babysit-prs: maintenance note still inside ## Pre-computed context (OPEN from first review)

babysit-prs/SKILL.md L15

Also not addressed. The gather instruction for branch and working tree remains inline in the ## Pre-computed context section as a plain-text note rather than in a dedicated ## Repository context — gather first section like the other four skills. This is harmless at runtime but inconsistent and easy for future editors to misread. Since the note is already two lines, moving it to a new section is line-neutral, which keeps babysit-prs within the 499-line cap.

---

Everything else looks correct

• Output-bounds restoration: The reading-rule approach is correct. Adding the pipes back would make them compound commands subject to the worktree guard — exactly the problem being fixed. Bounding at read time is the right alternative.
• CHANGELOG precision: The "what was verified, precisely" paragraph is a meaningful improvement over the first commit's wording. The distinction between "observed on unfixed skills" and "not exercised on edited skills (cache version does not exist yet)" is honest and important.
• commit/SKILL.md output bounds: git diff --cached --stat — summary (last) line only; git status --short — at most 20 entries. Both bounds are clearly stated and instructed not to be restored as pipes.
• resolve-conflicts/SKILL.md: git status — at most first 4 lines. Correctly noted that the in-progress banner is in those first 4 lines, explaining why that bound matters.
• worktree/SKILL.md: git worktree list — at most 30 entries. Bound is preserved at the same threshold as the original pipe.
• Version bump (0.33.1 → 0.33.2): Appropriate for a consumer-visible fix.

---

Summary: The sync commit (7cd4f1b) is a solid proactive improvement. Two findings from the first review remain unresolved — the test -f step needed to distinguish present but UNTRACKED from absent in commit/SKILL.md, and the babysit-prs gather instruction placement. Both are low-severity and non-blocking for the core fix.

[claude]

Claude finished @kyle-sexton's task in 1m 37s —— View job

---

Security Review — PR #1679

• Gather PR diff
• Read changed skill files
• Analyze shell command injection / path traversal surfaces
• Analyze trust-boundary and authorization gaps
• Analyze credential / secret handling
• Post findings

---

No exploitable security vulnerabilities found.

All nine changed files are markdown skill-instruction files and documentation. No executable scripts, GitHub Actions workflows, credential-handling code, or harness engine code is touched. The second commit (7cd4f1b — "restore the dropped output bounds and state the verification precisely") restores reading-bound instructions to the skill prose and tightens the verification statement in the CHANGELOG; neither change has security implications.

Path substitution in commit/SKILL.md

The decomposed config-layer probes (commit/SKILL.md L31–36) instruct the model to run git rev-parse --show-toplevel and substitute the result into the subsequent ls-files and test -f calls.

• Injection risk: effectively zero. Git normalizes --show-toplevel output to a canonical absolute filesystem path; it cannot contain shell metacharacters (;, `, $()). No attacker-controlled value reaches the path — the repository root is set by whoever created the worktree, not by repository content.
• Spaces-in-path degradation: an unquoted path with spaces would break the command. Per the "treat failure as unknown and carry on" instruction this degrades silently to "layer absent," not to an exploitable state. Not a security concern.

Decomposed body-level Bash calls

All commands moved from pre-compute into the skill body are read-only git introspection: branch --show-current, status --porcelain, log --oneline -5, diff --cached --stat, diff --name-only HEAD, diff --name-only --diff-filter=U, worktree list, rev-parse --git-dir, rev-parse --git-common-dir. None write to the repository or filesystem. Running them inside a worktree-isolated agent is the intended behavior; the trust model is unchanged.

Remaining pre-compute lines (commit, babysit-prs)

The two pre-compute blocks that stay are unchanged from main:

• bash "${CLAUDE_PLUGIN_ROOT}/skills/commit/scripts/exec-bit-check.sh" --probe — CLAUDE_PLUGIN_ROOT is a harness-set env var; the script path is fixed and the caller cannot influence it.
• gh api user --jq .login and gh pr list … --jq 'length' — read-only GitHub API calls scoped to the authenticated user; outputs are a username string and a count, respectively. No token or credential is echoed into skill context.

Loss of 2>/dev/null suppression

Old pre-compute lines suppressed stderr. The new prose says "treat a failure as an unknown value and carry on." Git error output on these read-only commands may include path or branch names, but no tokens, secrets, or credentials. Not a meaningful security surface.

pull-request/reference/create.md — --pushed mode rationale update

The revised rationale (create.md L513) correctly preserves the instruction to re-resolve from the target worktree explicitly rather than reusing the session cwd. The security-relevant behavior — not trusting the orchestrator's session context for an out-of-tree worker's branch state — is unchanged.

---

Verdict: Documentation-layer migration with no security regressions. All decomposed commands are read-only; no attacker-reachable values flow through the substituted paths; no credentials or sensitive data move across trust boundaries.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a1c221f48

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 1m 44s —— View job

---

PR Review (sync — post 9d35fa4)

• Gather context on updated PR
• Read commit/SKILL.md at HEAD to check open findings
• Assess whether all prior findings are resolved
• Post final review

---

What changed in 9d35fa4

9d35fa4 (fix(source-control): quote the substituted repository root in commit's config-layer probes) addresses the Codex P2 finding proactively before the review thread formally requested it. The change is correct:

• Both templates are now "<root>" (double-quoted).
• An explicit instruction is added: "Always double-quote the substituted path." with a concrete rationale (Windows paths with spaces, e.g. C:\Users\First Last\…).
• The CHANGELOG is updated to record the quoting fix.

The commit message also correctly notes that the unquoted git -C <worktree> templates in babysit-prs/reference are pre-existing and out of scope here — appropriate scoping.

---

Codex P2 — Shell-safe quoting: RESOLVED ✅

The quoting concern is fully addressed. The instruction at commit/SKILL.md L33–35 is unambiguous and explains the why, which is the right form for a model-facing instruction.

---

Finding 1 — commit: "present but UNTRACKED" still unreachable (OPEN — third round)

commit/SKILL.md L36–38

2. Tracked team layer — `git -C "<root>" ls-files --error-unmatch .claude/source-control.md`.
   Exit 0 means present **and tracked**. A file sitting at that path that this command does not
   report is `present but UNTRACKED`, and is deliberately not a config layer.

Still unresolved. The text asserts "A file sitting at that path" but gives the model no way to determine whether a file is sitting at that path when ls-files exits non-zero. A non-zero exit is produced identically by an absent file and by an untracked file — the model cannot distinguish them without an explicit test -f call. The downstream note at L50–52 still references present but UNTRACKED as a classification that produces a user warning, so the intent is intact but the path to reach it remains missing.

Required addition between step 2 and step 3:

If ls-files exits non-zero, additionally run test -f "<root>/.claude/source-control.md" to distinguish present but UNTRACKED (warn the user; do not use as a config layer) from absent.

Fix this →

---

Finding 2 — babysit-prs: maintenance note still inside ## Pre-computed context (OPEN — third round)

babysit-prs/SKILL.md L15

Still unresolved. The gather instruction for branch and working tree remains as a plain-text note inside the ## Pre-computed context block rather than in a dedicated ## Repository context — gather first section matching the other four skills. The note is two lines; moving it to a new section is line-neutral (keeping babysit-prs within the 499-line cap) and removes the ambiguity for future editors about whether this is a computed value.

---

Everything else looks correct

• Quoting fix is well-scoped: the commit correctly limits itself to commit/SKILL.md and does not attempt to chase the pre-existing unquoted templates in babysit-prs/reference — those are out of scope and the commit message says so explicitly.
• CHANGELOG update: the revised line ("quoted — a Windows repository root with a space in it word-splits otherwise") accurately records what was fixed and why.
• Core fix (worktree-isolation defect): remains correct and unaffected by 9d35fa4.

---

Summary: 9d35fa4 correctly resolves the Codex P2 shell-safety concern. Two findings from prior review rounds remain open — the test -f step needed to distinguish present but UNTRACKED from absent in commit/SKILL.md (Finding 1, third round), and the babysit-prs gather instruction placement (Finding 2, third round). Both are low-severity and non-blocking for the core worktree-isolation fix.

[claude]

Claude finished @kyle-sexton's task in 2m 1s —— View job

---

Security Review — PR #1679 (commit 9d35fa4)

• Gather PR diff for latest commit
• Read changed skill files
• Analyze path substitution / injection surfaces
• Analyze trust-boundary and authorization gaps
• Analyze credential / secret handling
• Post findings

---

No exploitable security vulnerabilities found.

All nine changed files are markdown skill-instruction files and documentation. No executable scripts, GitHub Actions workflows, credential-handling code, or harness engine code is touched. The third commit (9d35fa4 — "quote the substituted repository root in commit's config-layer probes") directly addresses the Codex reviewer's P2 finding from the previous round; the analysis below covers whether that fix is sufficient and what residual risk remains.

Path substitution in commit/SKILL.md — Codex finding addressed

The prior reviews noted that git rev-parse --show-toplevel output was substituted unquoted into the git -C and test -f templates. 9d35fa4 corrects both templates to "<root>" and adds an explicit instruction: "Always double-quote the substituted path."

The Codex finding is resolved for the stated scenario (spaces in the path, word-splitting). The residual question is whether double-quoting is sufficient against all metacharacters. On Linux/macOS, a directory name can technically contain $() or backticks (e.g., $(evil)), and double-quoting in bash does not suppress $() expansion. If a repository root were named with such characters, a model naively interpolating the raw string into a double-quoted bash argument could trigger expansion.

This is a theoretical concern rather than an exploitable one for two reasons:

1. The attacker must control the name of the directory the victim's git repository lives in — a scenario that requires local access already sufficient to do far worse.
2. The model constructs these commands interpretively, not via blind string interpolation; it is expected to handle unusual path characters as a competent shell user would (escaping, using a variable, etc.).

Assessment: The quoting guidance is the right instruction and eliminates the practical risk. The theoretical residual risk (attacker-controlled repo directory name containing $()) is out-of-scope for skill prose mitigation; it would require path sanitization at a lower layer.

Decomposed body-level Bash calls (all five skills)

All commands moved from pre-compute into the skill body are read-only git introspection: branch --show-current, status --porcelain / --short, log --oneline -5, diff --cached --stat, diff --name-only HEAD, diff --name-only --diff-filter=U, worktree list, rev-parse --git-dir, rev-parse --git-common-dir, rev-parse --show-toplevel, ls-files --error-unmatch. None write to the repository or filesystem. No trust-model change.

Remaining pre-compute lines (commit, babysit-prs) — unchanged from main

• bash "${CLAUDE_PLUGIN_ROOT}/skills/commit/scripts/exec-bit-check.sh" --probe — CLAUDE_PLUGIN_ROOT is a harness-set environment variable; not user-controlled; script path is fixed.
• gh api user --jq .login and gh pr list … --jq 'length' — read-only GitHub API calls; outputs are a username string and an integer count. No token, credential, or sensitive data echoed into skill context.

Trust boundary in pull-request/reference/create.md

The updated --pushed section (create.md L513) corrects the stated rationale while preserving the security-relevant instruction: do not trust the session cwd for an out-of-tree orchestrator; re-resolve from the target worktree explicitly. The behavior — rejecting the orchestrator's ambient context when constructing the PR — is unchanged. No regression.

Loss of 2>/dev/null stderr suppression

The new body instructions say "treat a failure as an unknown value and carry on." Git error output on these read-only introspection commands may include a path or branch name in the error text, but no tokens, secrets, or credentials. Not a meaningful exposure.

---

Verdict: Documentation-layer migration with no security regressions. The Codex-flagged unquoted-path issue is addressed in 9d35fa4. All decomposed commands are read-only; no attacker-reachable values flow through substituted paths beyond the theoretical self-inflicted directory-naming scenario noted above; no credentials or sensitive data cross trust boundaries.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9d35fa4b04

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 1m 43s —— View job

---

PR Review (sync — post 03457ca)

• Gather context on the updated PR
• Read latest commit changes
• Check Finding 1 — commit/SKILL.md: present but UNTRACKED reachability
• Check Finding 2 — babysit-prs/SKILL.md: maintenance note placement
• Assess new Codex P2 findings
• Post final review

---

What changed in 03457ca

03457ca ("use a portable placeholder in the Windows path example") is a one-line cosmetic change: C:\Users\First Last\… → C:\Users\<first last>\… in the quoting explanation in commit/SKILL.md L34. The original sample could be read as a real person's name; the angle-bracket form is unambiguously a placeholder. No behavioral change.

---

Finding 1 — commit: "present but UNTRACKED" still unreachable (OPEN — fourth round)

commit/SKILL.md L36–38

2. Tracked team layer — `git -C "<root>" ls-files --error-unmatch .claude/source-control.md`.
   Exit 0 means present **and tracked**. A file sitting at that path that this command does not
   report is `present but UNTRACKED`, and is deliberately not a config layer.

No change since the third round. The text asserts "A file sitting at that path" without giving the model a way to determine whether a file is there when ls-files exits non-zero. Both absent and present but UNTRACKED produce a non-zero exit; the model cannot distinguish them without an explicit test -f step. The downstream note at L50–52 still references present but UNTRACKED as a classification that produces a user warning, so the intent survives but the path to reach it is still absent.

Required addition between step 2 and step 3:

If ls-files exits non-zero, additionally run test -f "<root>/.claude/source-control.md" to distinguish present but UNTRACKED (warn the user; do not use as a config layer) from absent.

Fix this →

---

Finding 2 — babysit-prs: maintenance note still inside ## Pre-computed context (OPEN — fourth round)

babysit-prs/SKILL.md L15

Still unresolved. The gather instruction for branch and working tree remains as a plain-text note inside the pre-compute section instead of a dedicated ## Repository context — gather first section matching the other four skills. Moving it to a new section is line-neutral (the note is already two lines) and removes the ambiguity.

---

New Codex P2 findings (from 9d35fa4 review) — assessed below

Codex P2a — shell metacharacters beyond spaces (commit/SKILL.md L35)

The concern: a repository root containing $() or backticks would survive double-quoting and allow command expansion. The 03457ca commit does not address this; it changes only the placeholder text.

Assessment: the prior security review correctly scoped this as theoretical. The attack requires the victim to have a git repository rooted in a directory named with shell metacharacters (e.g., repo$(printf injected)), which in practice means the attacker already has write access to the filesystem — sufficient to do far worse. The model is also expected to construct commands interpretively rather than by blind string interpolation. This is not a mergeable blocker for this PR, but a follow-up note in the skill instructing the model to use a variable assignment (root="…") rather than inline substitution would close it more cleanly than the prose can.

Codex P2b — output caps applied at read time, not at output time (commit/SKILL.md L27)

The concern: git diff --cached --stat and git status --short emit their full output as a tool result before the model applies the reading bounds, potentially consuming context. The 7cd4f1b sync commit deliberately chose reading rules over pipes, because restoring | tail -1 / | head -20 would reintroduce a compound command — the exact defect being fixed. This tradeoff was already assessed in the third-round review as "deliberately a departure from the original truncation behavior" and is correct given the constraint. The analogous bounds in resolve-conflicts and worktree have the same shape for the same reason. Not a regression relative to main; the old truncation was applied at injection time precisely because the value was pre-computed. There is no equivalent mechanism in the body. This is a known, correctly documented limitation.

---

03457ca itself — looks correct

The placeholder change is accurate and unambiguous. <first last> makes clear this is a template slot, not a sample name. The instruction's meaning and the quoting rationale are unchanged.

---

Summary

03457ca is a clean, well-targeted cosmetic improvement. The two persistently open findings — the missing test -f step in commit/SKILL.md and the babysit-prs gather note placement — remain unresolved and are the only items standing between this PR and a clean review. Both are low-severity and non-blocking for the core worktree-isolation fix, but Finding 1 in particular does leave a documented behavior (user-visible present but UNTRACKED warning) unreachable.

[claude]

Claude finished @kyle-sexton's task in 2m 42s —— View job

---

Security Review — PR #1679 (commit 03457ca)

• Gather PR diff
• Read changed skill files at HEAD
• Analyze shell command injection / path substitution surfaces
• Analyze credential / secret handling
• Analyze trust-boundary and authorization gaps
• Post findings

---

No exploitable security vulnerabilities found.

All nine changed files are markdown skill-instruction files and documentation. The latest commit (03457ca) is prose-only: it replaces the literal First Last in a Windows path example with the placeholder <first last> to pass a hygiene gate that flags machine-specific absolute paths. No behavioral or security change.

---

Codex P2 (round 2) — $() expansion inside double quotes

The second Codex review raised that double-quoting the substituted repository root does not prevent command substitution ($() / backticks) if a directory name contains those characters. This is technically accurate for bash.

Assessment (unchanged from prior round): this is theoretical, not exploitable in practice.

• git rev-parse --show-toplevel emits a canonical absolute filesystem path. Paths containing $() or backticks are legal on Linux/macOS but effectively never occur outside deliberate adversarial construction.
• An attacker who controls the repository root directory name already has local code execution — sufficient to do far worse than influence a git config-layer probe.
• These instructions guide a model, not a shell interpreter directly; the model is expected to handle unusual characters the same way a competent shell user would (e.g., using printf '%q' or escaping), informed by the "always double-quote" instruction.
• The prior review round (after 9d35fa4) correctly categorized this residual as out-of-scope for skill prose mitigation — skill prose cannot fully bridge to the lower-layer shell-escaping guarantee, and there is no new exposure here vs. that round.

Codex's output-cap concern (round 2) — that git diff --cached --stat and git status --short emit full output before the reading bounds apply — is a functional/context concern, not a security one. No attacker-controlled data flows through the command output; the worst case is a large diff consuming model context.

---

create --pushed mode — worktree path handling

pull-request/reference/create.md L515–530

The example commands use "$WT" throughout (git -C "$WT", cd "$WT"). The security-relevant instruction — do not use the session cwd for an out-of-tree orchestrator — is preserved. The updated rationale correctly removes the now-stale claim that !-substituted lines cannot be git -C-redirected, while keeping the operative instruction on the reason that still holds.

The CLAUDE_PLUGIN_ROOT-scoped script path (bash "${CLAUDE_PLUGIN_ROOT}/skills/pull-request/scripts/resolve-remote.sh" --push) is a fixed path in a harness-set env var; no user-controlled values flow into it.

---

Remaining pre-compute lines — unchanged from main

• bash "${CLAUDE_PLUGIN_ROOT}/skills/commit/scripts/exec-bit-check.sh" --probe — harness-set env var, fixed script path, no user-controlled input.
• gh api user --jq .login — read-only; fixed jq filter; output is a username string.
• gh pr list --state open --author "@me" --limit 200 --json number --jq 'length' — read-only; fixed jq filter; output is an integer count. No token or credential echoed into skill context.

---

Decomposed body-level Bash calls (all five skills)

All commands moved from pre-compute into the skill body are read-only git introspection: branch --show-current, status --porcelain / --short, log --oneline -5, diff --cached --stat, diff --name-only HEAD, diff --name-only --diff-filter=U, worktree list, rev-parse --git-dir, rev-parse --git-common-dir, rev-parse --show-toplevel, ls-files --error-unmatch. None write to the repository or filesystem. No trust-model change.

Loss of 2>/dev/null suppression

The new body instructions say "treat failure as unknown and carry on." Git error output on these read-only introspection commands may include path or branch names, but contains no tokens, secrets, or credentials. Not a meaningful security surface.

babysit-prs tier authorization model

The merge-capable worker and autopilot tiers continue to require explicit keyword opt-in. Moving the git gather out of pre-compute does not affect this logic. The default_tier: safe never merges. Authorization model unchanged.

---

Verdict: Documentation-layer migration with no security regressions across all four commits. The Codex P2 $() residual is theoretical, correctly categorized as out-of-scope for skill prose, and is not new to this round. The latest commit (03457ca) is prose-only with no behavioral surface.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 03457ca34b

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 36s —— View job

---

PR Review (sync — post f92e785)

• Gather context on updated PR
• Read commit/SKILL.md at HEAD — check Finding 1 (test -f / present but UNTRACKED)
• Read babysit-prs/SKILL.md at HEAD — check Finding 2 (maintenance note placement)
• Assess new commit changes
• Post final review

---

What changed in f92e785

f92e785 ("anchor config probes on the :/ pathspec and restore real output bounds") addresses all three outstanding P2 items in a single commit across commit/SKILL.md, worktree/SKILL.md, resolve-conflicts/SKILL.md, and the CHANGELOG.

---

Finding 1 — commit: "present but UNTRACKED" reachability: RESOLVED ✅

commit/SKILL.md L38–56

The fix goes beyond what the review asked for. Instead of adding a test -f step (which would still require substituting the root path and couldn't detect gitignored files), the three-probe approach eliminates root substitution entirely:

1. git ls-files --error-unmatch -- ":/.claude/source-control.md" — trackedness
2. git ls-files --cached --others -- ":/.claude/source-control.md" — existence
3. git ls-files --cached --others -- ":/.claire/source-control.local.md" — personal overlay

The combining table is explicit and correct:

Trackedness (1)	Existence (2)	Team-layer state
exit 0	not consulted	present (tracked)
nonzero	non-empty output	present but UNTRACKED
nonzero	empty output	absent

Crucially, --exclude-standard is deliberately omitted from probes 2 and 3, so a gitignored file is still listed — the test -f approach could not have achieved this. The generic "treat failure as unknown" rule is also correctly narrowed: a nonzero exit from probe 1 is a result, not a failure.

---

Codex P2 — Shell metacharacter injection: RESOLVED ✅

commit/SKILL.md L33–47

The :/ magic pathspec eliminates root substitution entirely. A repository root containing $(…), backticks, spaces, or double quotes can neither break the command nor inject into it, because the root never enters the command string. This is the correct fix — as the commit message notes, double-quoting a substituted root does not neutralize command substitution ($() evaluates inside double quotes), so quoting was the wrong approach. The CHANGELOG records this clearly: "double-quoting a substituted root does not neutralize a command substitution, which is why quoting was the wrong fix."

---

Codex P2 — Output bounds: RESOLVED ✅ (real pipes restored)

commit/SKILL.md L21–22, worktree/SKILL.md, resolve-conflicts/SKILL.md

The | tail -1 and | head -20 pipes are restored as actual bounds on the body Bash calls (not just prose instructions). The commit message explains why: a read-time prose cap bounded nothing, because the Bash tool returns complete output into context before there is anything to decide about. The rationale for why restoring them as body pipes is safe (not the pre-compute composition shape that #1619 is about) is correct and now stated explicitly in each skill.

One honest caveat in the CHANGELOG: whether a pipe in a body call also clears the isolation guard hasn't been verified. Based on the established mechanism (the guard fires on git inside a composed pre-compute block, not on individual body calls), body-level pipes should be fine — but this is the right thing to disclose.

---

Finding 2 — babysit-prs: maintenance note still inside ## Pre-computed context (OPEN — fifth round)

babysit-prs/SKILL.md L15

Still unresolved. The f92e785 commit does not touch babysit-prs/SKILL.md. The gather instruction for branch and working tree remains as a plain-text note inside ## Pre-computed context rather than a dedicated ## Repository context — gather first section matching the other four skills. The note is already two lines; moving it to a new section is line-neutral under the 499-line cap.

Fix this →

---

Everything else looks correct

• :/ pathspec rationale in ## Repository context: The note at L79–86 explains precisely why :/ retires the older two-step and why double-quoting was wrong. This is the right level of explanation for a model-facing instruction — the "why" prevents future regressions.
• CHANGELOG revision: The updated entry correctly records all three changes, including the honest caveat about body-pipe isolation verification. The "real bound beats a fictional one" line is accurate.
• worktree/SKILL.md and resolve-conflicts/SKILL.md: Both now have the pipe restored and carry the same "pipe is the bound" rationale paragraph. Consistent with commit/SKILL.md.
• Core fix (worktree-isolation): Unaffected by f92e785. All five skills remain correct for the primary defect.
• Version bump (0.33.1 → 0.33.2): Appropriate.

---

Summary: f92e785 is a thorough and well-reasoned resolution of all three P2 findings. Finding 1 (team-layer three-state check) is resolved with a superior approach. The only item still open is Finding 2 — the babysit-prs gather note placement — which is low-severity and non-blocking for the core fix.

[claude]

Claude finished @kyle-sexton's task in 2m 5s —— View job

---

Security Review — PR #1679 (commit f92e785)

• Gather PR diff
• Read changed skill files at HEAD
• Analyze shell command injection / path substitution surfaces
• Analyze trust-boundary and authorization gaps
• Analyze credential / secret handling
• Post findings

---

No exploitable security vulnerabilities found.

All nine changed files are markdown skill-instruction files and documentation. No executable scripts, GitHub Actions workflows, credential-handling code, or harness engine code is touched.

The final commit (f92e785 — "anchor config probes on the :/ pathspec and restore real output bounds") is the most security-relevant change in this PR. The analysis below covers each surface.

---

Codex P2 — shell metacharacter injection via substituted repository root: FULLY RESOLVED

The commit eliminates the substitution surface entirely rather than trying to sanitize it. The old two-step approach (git rev-parse --show-toplevel, then substitute the literal path into git -C "$R" and test -f "$R/…") was replaced with git's repo-root-relative magic pathspec:

git ls-files --error-unmatch -- ":/.claude/source-control.md"
git ls-files --cached --others -- ":/.claude/source-control.md"
git ls-files --cached --others -- ":/.claude/source-control.local.md"

• Nothing is substituted. A repository root containing $(…), a backtick, or a double quote cannot be injected — the pathspec argument is a hardcoded string literal.
• The -- separator prevents any pathspec from being parsed as a flag.
• :/ resolves against the working-tree root regardless of session cwd — the same correctness guarantee the old approach achieved via explicit root resolution, without the injection surface.

The CHANGELOG explicitly documents why quoting was the wrong fix: "double-quoting a substituted root does not neutralize a command substitution, which is why quoting was the wrong fix." The :/ approach is the correct fix — it removes the substitution entirely.

--exclude-standard omission on existence probes

Probes 2 and 3 (git ls-files --cached --others -- ":/…") deliberately omit --exclude-standard, making gitignored files visible to the command. This is intentional and documented: the personal overlay is expected to be gitignored, and an untracked team file must be seen before it can be classified as untracked rather than absent.

The output of these probes is only the matched filename — no file content. The paths are fixed at two specific well-known config file locations. No attacker-controlled value determines which paths are queried, and the result (filename present or absent) is information the model would obtain anyway by attempting to read the file. No exposure.

Decomposed body-level Bash calls (all five skills)

All commands moved from pre-compute into the skill body are read-only git introspection: branch --show-current, status --porcelain / --short, log --oneline -5, diff --cached --stat, diff --name-only HEAD, diff --name-only --diff-filter=U, worktree list, rev-parse --git-dir, rev-parse --git-common-dir, ls-files --error-unmatch, ls-files --cached --others. None write to the repository or filesystem. No trust-model change.

Restored output-cap pipes

git diff --cached --stat | tail -1, git status --short | head -20, git worktree list | head -30, git status | head -4 are now real pipe operations on body calls. These are single-purpose bounded reads, not compound commands with side effects. No security surface.

Remaining pre-compute lines — unchanged from main

• bash "${CLAUDE_PLUGIN_ROOT}/skills/commit/scripts/exec-bit-check.sh" --probe — CLAUDE_PLUGIN_ROOT is harness-set; the script path is fixed and not user-reachable.
• test -f "$HOME/.claude/source-control.md" — HOME is OS-set; output is the string present or absent; no file content.
• gh api user --jq .login — read-only GitHub API, fixed jq filter; output is a username string.
• gh pr list --state open --author "@me" --limit 200 --json number --jq 'length' — read-only GitHub API, fixed jq filter; output is an integer count. No token, credential, or sensitive data echoed into skill context.

pull-request/reference/create.md — --pushed mode trust boundary

The updated rationale (create.md L513) correctly preserves the security-relevant behavior: do not trust the session cwd for an out-of-tree orchestrator; re-resolve from the target worktree explicitly. The stale claim ("a !-substituted line cannot be git -C-redirected") is removed and replaced with the reason that still holds. The operative instruction is unchanged.

Loss of 2>/dev/null stderr suppression

New body instructions say "treat a failure as an unknown value and carry on." Git error output on these read-only introspection commands may include a path or branch name in error text, but no tokens, secrets, or credentials. Not a meaningful exposure.

---

Verdict: Documentation-layer migration with a net security improvement. The final commit (f92e785) correctly resolves the Codex P2 finding — not by sanitizing the substitution, but by eliminating it. The :/ pathspec approach closes the injection surface completely. All decomposed commands are read-only; no user-controlled values flow through any argument; no credentials or sensitive data cross trust boundaries.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f92e785007

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kyle-sexton]

Correction to this PR's own framing — read before merging

This PR body states the mechanism as "the harness composes an entire pre-compute block into ONE shell invocation, and the guard refuses a git-bearing compound command." That is wrong, and it was my error, not a reviewer's.

An adversarial re-probe, plus a confirmation run from inside an isolation: "worktree" agent, established:

A command is refused iff it contains a $-expansion in any form other than bare $HOME or "$HOME". Git, pipes, 2>/dev/null, ||, and multi-line composition are irrelevant.

Decisive controls: git status --porcelain 2>/dev/null | head -20 || echo clean passes under isolation, while a git-free echo "${CLAUDE_CODE_SESSION_ID:-unknown}" refuses. source-control:worktree — four git pre-compute lines, no $ — loads fine unfixed, which is the negative control I should have built before publishing the first diagnosis.

Full evidence: #1619 correction.

What this means for merging

The changes here are still worth landing. Moving git into individual body calls is sound, the :/ magic-pathspec rewrite genuinely removes a shell-injection class, the three-state config-layer probe fixes a real contract break, and the restored pipes fix a bound that was fictional. None of that depends on the wrong mechanism.

But this PR does not fully fix commit. Two pre-compute lines survive and both carry $-expansion:

Exec-bit backstop: !`bash "${CLAUDE_PLUGIN_ROOT}/skills/commit/scripts/exec-bit-check.sh" --probe …`
Config layer (user-global): !`test -f "$HOME/.claude/source-control.md" && echo present || echo absent`

Under the corrected rule, ${CLAUDE_PLUGIN_ROOT} still refuses, so /source-control:commit remains uninvocable from a worktree-isolated agent after this merges. The $HOME line is probably fine (bare $HOME passes) but sits on an uncharacterized allowlist.

That remainder is tracked in #1687, which also notes the hard part: ${CLAUDE_PLUGIN_ROOT} is the documented way a plugin references its own bundled scripts, so there may be no clean in-plugin fix — that case may be genuinely harness-blocked rather than something this repo can resolve.

Merging on the strength of what the diff actually improves, with the residue tracked rather than papered over. The CHANGELOG's mechanism wording carries the same inherited error and should be corrected in the #1687 PR alongside the real fix.

[kyle-sexton]

Correcting my own correction — the commit residue claim above is probably WRONG

Earlier in this thread I wrote that /source-control:commit "remains uninvocable from a worktree-isolated agent after this merges" because of its surviving pre-compute lines:

Exec-bit backstop: !`bash "\/skills/commit/scripts/exec-bit-check.sh" --probe …`
Config layer (user-global): !`test -f "\/c/Users/KyleSexton/.claude/source-control.md" && echo present || echo absent`

A subsequent probe found the premise false. \ is substituted by the harness into a literal path before any shell sees it — in pre-compute as well as in body prose. There is no $ left for the isolation guard to catch.

Evidence: printenv CLAUDE_PLUGIN_ROOT returns exit 1, no value (the variable is never in the Bash tool's environment); a skill body reference rendered as C:/Users/<user>/.claude/plugins/cache/melodic-software/claude-config/0.14.0/skills/…; and three skills whose pre-compute uses only plugin variables loaded successfully under isolation, one of them returning real script output rather than its || echo fallback.

So both surviving lines here are in the passing class, and this PR most likely did fully fix commit.

I am marking that as inferred, not confirmed — source-control:commit itself was not directly invoked from an isolated agent, and the installed cache is still at 0.33.1, one version behind this merge. The honest state is: my "still broken" claim is unsupported, and the opposite is likely but untested.

Full class analysis on #1687. Nothing here needs reverting — the diff is unaffected either way; only my characterization of the residue was wrong.