Issue Station
Sign In
Tools hereToolbox

openclaw - ✅(Solved) Fix DX: `git-hooks/pre-commit` fails on merge commits when staged content includes `.gitignore`-newly-ignored paths [1 pull requests, 1 comments, 2 participants]

Official PRs (…)
ON THIS PAGE

Recommended Tools

×6

Utilities matched from this issue’s tags and category — try them while you read without losing context.

GitHub issue graph ai analysis

Paste a GitHub issue URL. We fetch that issue, discover linked issues from bodies/comments/timeline, collect linked pull requests, and produce a structured English report.

The report is written in English Markdown for sharing and archival.

Helpful · Quick feedback

Loading…
GitHub stats
openclaw/openclaw#72744•Fetched 2026-04-28 06:32:37
View on GitHub
Comments
1
Participants
2
Timeline
11
Reactions
0
Author
100yenadmin100yenadmin
Participants
100yenadmin100yenadminsteipetesteipete
Timeline (top)
referenced ×8closed ×1commented ×1cross-referenced ×1

git-hooks/pre-commit (active via core.hooksPath = git-hooks) ends with an unconditional git add over every staged file. When a merge commit brings in content that the merged-in .gitignore rules now exclude (e.g. a sub-skill directory that's gitignored on main), the git add step fails with paths are ignored by .gitignore and the merge commit silently aborts. --no-verify is the only way through.

Error Message

git status afterwards shows the merge state still active and no commit was made. There's no error output beyond the gitignore hint, which is easy to miss.

Root Cause

git-hooks/pre-commit lines 36-38:

files=()
while IFS= read -r -d '' file; do
  files+=("$file")
done < <(git diff --cached --name-only --diff-filter=ACMR -z)
# ... format-eligible filtering ...
git add -- "${files[@]}"     # <-- fails on gitignored paths in $files

git diff --cached returns staged files including paths that git add would now refuse (because .gitignore matches them). Re-git adding them is unnecessary unless the formatter modified them on disk; even when needed, gitignored paths should be filtered out first.

Fix Action

Fixed

PR fix notes

PR #72276: [codex] Consolidate embedded runner structural splits

Description (problem / solution / changelog)

@steipete @pashpashpash

Summary

This PR is the single maintainer-facing RuntimePlan finalization and embedded-runner structural cleanup package for RFC #72072. It consolidates the earlier cleanup package plus the follow-up structural split PRs, then applies the actionable bot-review fixes from those PRs.

GitHub shows a cumulative diff against main; the tables below preserve the original PR/commit traceability so maintainers can review by logical slice instead of by archeology.

Superseded PRs

Superseded PRRole
#72134Consolidated RuntimePlan finalization cleanup package
#72259Split attempt prompt and transport preparation
#72261Extract embedded run attempt factory
#72269Extract attempt stream wrappers and diagnostic lifecycle
#72272Extract embedded run lane/workspace helpers, auto-closed by active PR limit
#72274Extract embedded run terminal result shaping, auto-closed by active PR limit

Commit Stack Map

CommitSourceScope
39b0afb525#72134Capture finalization baseline doc.
e754112b25#72134Add native AgentHarnessV2 factory registry.
fe4f272510#72134Extract attempt tool-policy helpers.
ecf2cd44fc#72134Extract attempt message summary helpers.
0689e41814#72134Extract run orchestration helpers.
755616970b#72134Add canonical/deprecated embedded-runner barrels.
9c6a61dc07#72134Add RuntimePlan finalization handoff doc.
83a1f03b52, 7dca2ad3e8, ae5a5cb7bb#72134Stabilization/review fixes and QA retrigger.
1cdfd2c5be#72259Split prompt/bootstrap-context and transport setup out of attempt.ts.
cc8c5f289f#72261Extract embedded run attempt input / RuntimePlan factory from run.ts.
96e08f50a5#72269Extract run diagnostic lifecycle emitter.
d1bf5e6e1a#72269Extract ordered stream wrapper stack from attempt.ts.
5b54353fc1#72269Relax diagnostic lifecycle params to match narrow production/test callers.
7b50588a14#72272Extract lane/workspace/channel-format/abort helpers from run.ts.
556cad03df#72274Extract success terminal-result shaping from run.ts.
c961ad9c25Review loopFix actionable bot comments from the superseded PRs.
42d927d7d7CI fixStringify the prior transport before logging transport overrides.
58ff685083Review loopRemove redundant attempt wrapper imports flagged on the consolidated PR.

Architecture

flowchart TD
  RFC["RFC #72072"] --> Contracts["RuntimePlan cleanup package (#72134)"]
  Contracts --> Harness["AgentHarnessV2 native factory registry"]
  Contracts --> Barrels["embedded-runner compatibility barrels"]
  Contracts --> Docs["baseline + handoff docs"]

  Harness --> AttemptPrep["attempt prompt + transport helpers"]
  AttemptPrep --> AttemptRuntime["attempt lifecycle + stream wrappers"]
  AttemptRuntime --> RunFactory["run attempt factory"]
  RunFactory --> LaneWorkspace["run lane/workspace helpers"]
  LaneWorkspace --> TerminalResult["run terminal result helper"]

  AttemptPrep --> AttemptTs["attempt.ts thinner orchestration"]
  AttemptRuntime --> AttemptTs
  RunFactory --> RunTs["run.ts thinner orchestration"]
  LaneWorkspace --> RunTs
  TerminalResult --> RunTs

File Map

File / AreaChange
docs/refactor/runtime-plan-finalization-baseline.mdDocuments baseline health and known drift before structural cleanup.
docs/refactor/runtime-plan-finalization-complete.mdMaintainer handoff for completed/deferred RuntimePlan finalization work.
src/agents/harness/v2.ts and harness testsAdds internal native V2 factory registry while preserving public V1 compatibility. Review fix: test comments now rely on explicit cleanup, not Vitest isolation.
src/agents/embedded-runner/index.ts, src/agents/pi-embedded-runner/index.ts, alias testsAdds neutral embedded-runner barrels while keeping Pi-named compatibility aliases.
src/agents/pi-embedded-runner/run/attempt-tools.tsTool allow-list/policy helper extraction.
src/agents/pi-embedded-runner/run/attempt-message-summary.tsDiagnostic message/transcript summary helper extraction.
src/agents/pi-embedded-runner/run/attempt-prompt.tsBootstrap routing/context injection and prompt-boundary preparation helper.
src/agents/pi-embedded-runner/run/attempt-transport.tsPer-turn streamFn, transport override, text-transform, extra-param, and prompt-cache-retention helper. Review fix: streamFn is required and activeSession.sessionId is the single source of truth.
src/agents/pi-embedded-runner/run/attempt-lifecycle.tsDiagnostic run.started / once-only run.completed lifecycle emitter. Review fix: hoisted mock and aborted outcome coverage.
src/agents/pi-embedded-runner/run/attempt-stream-wrappers.tsOrdered stream wrapper stack for cache tracing, transcript sanitation, yield abort, malformed tool-call cleanup/repair, payload logging, and stop-reason recovery.
src/agents/pi-embedded-runner/run/runtime-plan-factory.tsAttempt input builder and RuntimePlan wiring from run.ts.
src/agents/pi-embedded-runner/run/lane-workspace.tsQueue lane planning, tool-result format selection, probe-session detection, abort normalization, workspace context resolution, and fallback logging. Review fix: documents that caller-supplied enqueue opts out of lane routing.
src/agents/pi-embedded-runner/run/terminal-result.tsSuccess terminal EmbeddedPiRunResult shaping, stop-reason priority, execution trace, request shaping, completion trace, pending hosted tool calls, and silent-empty payloads.
src/agents/pi-embedded-runner/run.ts, run/attempt.tsDelegates extracted seams while keeping core run/recovery behavior in place.

Review Comments Addressed

SourceFix
#72134 Copilot commentReworded V2 registry test comment to avoid claiming Vitest test-file isolation; explicit cleanup is the invariant.
#72259/#72261 Copilot commentsMade AttemptTransportSession.agent.streamFn required and removed duplicated sessionId parameter.
#72269 Greptile/Copilot commentsAdded aborted lifecycle diagnostic test and switched to vi.hoisted mock setup.
#72272 Greptile commentAdded comment documenting that caller-provided enqueue opts out of lane routing and the cron deadlock guard.
#72276 Copilot commentRemoved redundant direct imports from attempt.ts; the existing re-export statements now carry those helpers alone.
#72276 check-lintFixed the restrict-template-expressions failure in attempt-transport.ts by normalizing the previous transport value before interpolation.

What Changed

  • The RuntimePlan finalization cleanup docs/barrels/Harness V2 registry and the structural helper extractions are in one package.
  • attempt.ts and run.ts are thinner by ownership boundary, not cosmetic convenience.
  • Extracted helpers have focused tests for behavior that used to be buried in long functions.
  • The most closure-heavy retry/error recovery paths remain inline to avoid behavioral drift.

What Did Not Change

  • No public plugin API removal.
  • No Harness V2 public plugin API expansion.
  • No pi-embedded-runner package rename beyond compatibility barrels.
  • No WS pooling.
  • No retry/error recovery extraction from the stream loop.
  • No work on prototype PRs #70743 or #70772.

Validation

Passed locally on this branch:

node scripts/run-vitest.mjs run --config test/vitest/vitest.agents.config.ts \
  src/agents/harness/v2.test.ts --reporter verbose

./node_modules/.bin/vitest run --config test/vitest/vitest.unit-fast.config.ts \
  src/agents/pi-embedded-runner/run/terminal-result.test.ts --reporter verbose

node scripts/run-vitest.mjs run --config test/vitest/vitest.agents.config.ts \
  src/agents/pi-embedded-runner/run/attempt-lifecycle.test.ts \
  src/agents/pi-embedded-runner/run/attempt.test.ts \
  src/agents/pi-embedded-runner/run/attempt.tool-call-normalization.test.ts \
  src/agents/pi-embedded-runner/run/llm-idle-timeout.test.ts \
  src/agents/pi-embedded-runner/run/lane-workspace.test.ts --reporter verbose

./node_modules/.bin/oxlint --tsconfig tsconfig.oxlint.core.json \
  src/agents/harness/v2.test.ts \
  src/agents/pi-embedded-runner/run/attempt.ts \
  src/agents/pi-embedded-runner/run/attempt-transport.ts \
  src/agents/pi-embedded-runner/run/attempt-lifecycle.test.ts \
  src/agents/pi-embedded-runner/run/lane-workspace.ts \
  src/agents/pi-embedded-runner/run.ts \
  src/agents/pi-embedded-runner/run/terminal-result.ts \
  src/agents/pi-embedded-runner/run/terminal-result.test.ts

git diff --check
pnpm check:architecture

OPENCLAW_OXLINT_SKIP_LOCK=1 OPENCLAW_OXLINT_SKIP_PREPARE=1 \
  node scripts/run-oxlint.mjs --tsconfig tsconfig.oxlint.core.json src ui packages --threads=8

Known current-main / stack baseline:

pnpm check:test-types

Still fails on existing ModelCompatConfig.supportsLongCacheRetention / missing @vincentkoc/qrcode-tui drift. No files introduced by this PR appear in the remaining failure list.

Deferred Work

  • Model/auth plan extraction from run.ts if maintainers want another structural slice.
  • Full neutral embedded-runner import guard after compatibility aliases settle.
  • Final GPT-5.4 smoke/handoff doc after this package lands.

Changed files

  • docs/refactor/runtime-plan-finalization-baseline.md (added, +154/-0)
  • docs/refactor/runtime-plan-finalization-complete.md (added, +123/-0)
  • extensions/feishu/src/outbound.test.ts (modified, +8/-7)
  • extensions/feishu/src/outbound.ts (modified, +11/-7)
  • src/agents/embedded-runner/index.ts (added, +31/-0)
  • src/agents/harness/builtin-pi.test.ts (added, +106/-0)
  • src/agents/harness/builtin-pi.ts (modified, +51/-2)
  • src/agents/harness/selection.test.ts (modified, +32/-0)
  • src/agents/harness/selection.ts (modified, +2/-2)
  • src/agents/harness/v2.test.ts (modified, +128/-1)
  • src/agents/harness/v2.ts (modified, +42/-0)
  • src/agents/pi-embedded-runner/aliases.test.ts (modified, +27/-0)
  • src/agents/pi-embedded-runner/index.ts (added, +42/-0)
  • src/agents/pi-embedded-runner/run.ts (modified, +111/-412)
  • src/agents/pi-embedded-runner/run/attempt-lifecycle.test.ts (added, +89/-0)
  • src/agents/pi-embedded-runner/run/attempt-lifecycle.ts (added, +78/-0)
  • src/agents/pi-embedded-runner/run/attempt-message-summary.ts (added, +90/-0)
  • src/agents/pi-embedded-runner/run/attempt-prompt.ts (added, +165/-0)
  • src/agents/pi-embedded-runner/run/attempt-stream-wrappers.test.ts (added, +369/-0)
  • src/agents/pi-embedded-runner/run/attempt-stream-wrappers.ts (added, +227/-0)
  • src/agents/pi-embedded-runner/run/attempt-tools.ts (added, +155/-0)
  • src/agents/pi-embedded-runner/run/attempt-transport.ts (added, +177/-0)
  • src/agents/pi-embedded-runner/run/attempt.ts (modified, +84/-655)
  • src/agents/pi-embedded-runner/run/lane-workspace.test.ts (added, +149/-0)
  • src/agents/pi-embedded-runner/run/lane-workspace.ts (added, +135/-0)
  • src/agents/pi-embedded-runner/run/run-orchestration-helpers.ts (added, +121/-0)
  • src/agents/pi-embedded-runner/run/runtime-plan-factory.test.ts (added, +96/-0)
  • src/agents/pi-embedded-runner/run/runtime-plan-factory.ts (added, +244/-0)
  • src/agents/pi-embedded-runner/run/terminal-result.test.ts (added, +183/-0)
  • src/agents/pi-embedded-runner/run/terminal-result.ts (added, +162/-0)
  • src/gateway/server-chat.stream-text-merge.test.ts (modified, +1/-4)
  • src/gateway/server/health-state.test.ts (modified, +3/-1)
  • ui/src/ui/app-channels.test.ts (modified, +5/-2)

Code Example

git merge origin/main           # auto-stages 800+ merged files including the discord-clawd dir
# resolve manual conflicts in run.ts and attempt.ts, git add them
git commit -m "..."             # pre-commit hook runs
# -> [oxfmt finishes]
# -> The following paths are ignored by one of your .gitignore files:
# ->   .agents/skills/discord-clawd
# -> hint: Use -f if you really want to add them.
# -> (silent exit, commit does NOT happen)

---

files=()
while IFS= read -r -d '' file; do
  files+=("$file")
done < <(git diff --cached --name-only --diff-filter=ACMR -z)
# ... format-eligible filtering ...
git add -- "${files[@]}"     # <-- fails on gitignored paths in $files

---

non_ignored=()
while IFS= read -r -d '' file; do
  if ! git check-ignore -q -- "$file"; then
    non_ignored+=("$file")
  fi
done < <(printf '%s\0' "${files[@]}")

if [ "${#non_ignored[@]}" -gt 0 ]; then
  git add -- "${non_ignored[@]}"
fi
RAW_BUFFERClick to expand / collapse

Summary

git-hooks/pre-commit (active via core.hooksPath = git-hooks) ends with an unconditional git add over every staged file. When a merge commit brings in content that the merged-in .gitignore rules now exclude (e.g. a sub-skill directory that's gitignored on main), the git add step fails with paths are ignored by .gitignore and the merge commit silently aborts. --no-verify is the only way through.

Reproduction

While merging origin/main into a PR branch (here: contract-finalization/run-terminal-result-split for PR #72276), the merge auto-staged .agents/skills/discord-clawd/SKILL.md and .agents/skills/discord-clawd/agents/openai.yaml (these came in with origin/main's content) but a .gitignore rule now excludes that directory.

git merge origin/main           # auto-stages 800+ merged files including the discord-clawd dir
# resolve manual conflicts in run.ts and attempt.ts, git add them
git commit -m "..."             # pre-commit hook runs
# -> [oxfmt finishes]
# -> The following paths are ignored by one of your .gitignore files:
# ->   .agents/skills/discord-clawd
# -> hint: Use -f if you really want to add them.
# -> (silent exit, commit does NOT happen)

git status afterwards shows the merge state still active and no commit was made. There's no error output beyond the gitignore hint, which is easy to miss.

Root cause

git-hooks/pre-commit lines 36-38:

files=()
while IFS= read -r -d '' file; do
  files+=("$file")
done < <(git diff --cached --name-only --diff-filter=ACMR -z)
# ... format-eligible filtering ...
git add -- "${files[@]}"     # <-- fails on gitignored paths in $files

git diff --cached returns staged files including paths that git add would now refuse (because .gitignore matches them). Re-git adding them is unnecessary unless the formatter modified them on disk; even when needed, gitignored paths should be filtered out first.

Fix shape

Filter staged paths through git check-ignore before the final git add:

non_ignored=()
while IFS= read -r -d '' file; do
  if ! git check-ignore -q -- "$file"; then
    non_ignored+=("$file")
  fi
done < <(printf '%s\0' "${files[@]}")

if [ "${#non_ignored[@]}" -gt 0 ]; then
  git add -- "${non_ignored[@]}"
fi

Or, equivalently, pass --ignore-errors to git add (less surgical — masks unrelated failures too).

Verification

After fixing, attempt a merge of origin/main into a branch behind by 800+ commits (or any branch where the merge brings in .gitignore-newly-ignored paths) and git commit without --no-verify should succeed.

Impact

  • Any merge commit that crosses a .gitignore rule update silently fails to commit. The user/agent has to choose --no-verify or unstage the offending paths (which loses content from the merge).
  • Cross-fork PR merges are particularly likely to trip this — the .gitignore rules in the destination repo can drift while the fork keeps the older content.
  • DX issue, not a correctness/security issue. bug:behavior.

Discovered

Driving PR #72276 to mergeable. Used --no-verify on the merge commit (aadc069342) only after verifying pnpm check:changed was clean (so the hook's lint/format purpose was already satisfied via a separate green run); commit message documents the bypass and rationale.

extent analysis

TL;DR

Filter out gitignored paths before re-adding files in the git-hooks/pre-commit script to prevent silent merge commit failures.

Guidance

  • Identify the git-hooks/pre-commit script and locate the lines responsible for re-adding staged files.
  • Modify the script to filter out gitignored paths using git check-ignore before re-adding files, as shown in the proposed fix shape.
  • Alternatively, consider passing --ignore-errors to git add, but be aware that this may mask unrelated failures.
  • Verify the fix by attempting a merge of origin/main into a branch behind by 800+ commits and checking that git commit succeeds without --no-verify.

Example

The proposed fix shape code snippet can be used as a replacement for the existing git add line in the git-hooks/pre-commit script:

non_ignored=()
while IFS= read -r -d '' file; do
  if ! git check-ignore -q -- "$file"; then
    non_ignored+=("$file")
  fi
done < <(printf '%s\0' "${files[@]}")

if [ "${#non_ignored[@]}" -gt 0 ]; then
  git add -- "${non_ignored[@]}"
fi

Notes

This fix assumes that the git-hooks/pre-commit script is the only place where files are being re-added. If there are other scripts or hooks that also re-add files, they may need to be modified as well.

Recommendation

Apply the workaround by filtering out gitignored paths in the git-hooks/pre-commit script, as this is a more targeted and safer solution than passing --ignore-errors to git add.

Vote matrix · Quick signals

Works
Did the solution work? Tap to confirm.
Easy Fix
Was it a quick fix?
Time Saver
Did it save you time?
Blocking
Was it severely blocking?
Common Issue
Are others likely hitting this too?
Flaky / Intermittent
Is it intermittent?
Verified / Reproducible
Can you reproduce it reliably?
Loading…
#tool integration#LLM response#prompt template#agent execution#callback error

Still need to ship something?

×6

Another batch ranked right after the header list — different links, same matching logic.

Back to top recommendations

TRENDING

openclaw - ✅(Solved) Fix DX: `git-hooks/pre-commit` fails on merge commits when staged content includes `.gitignore`-newly-ignored paths [1 pull requests, 1 comments, 2 participants]