Conversation
Summary by CodeRabbitRelease Notes
WalkthroughBumps Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 3964fb8914
ℹ️ 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".
src/prop-bag.js
Outdated
| if (typeof props.size === 'number') { | ||
| return props.size; |
There was a problem hiding this comment.
size field
getPropSize currently treats any property bag with a numeric size field as Map-like. With the new object-shaped getNodeProps() payloads, nodes can legitimately have a numeric size property (for example from YAML import), so an object like { size: 0, status: 'todo' } is misclassified as empty. Because exportGraph and renderView both gate property handling on getPropSize(propsMap) > 0, those nodes lose all exported/view properties in this case. Please detect Map semantics explicitly (for example via instanceof Map or method checks) before trusting props.size.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/export.js (1)
88-89:⚠️ Potential issue | 🟡 MinorApply
getPropEntriesabstraction to edge properties, not just node properties.Edge properties at lines 88–89 use raw
Object.entries(edge.props), while node properties (line 64) use thegetPropEntries()abstraction. The abstraction exists explicitly because git-warp v14+ has a documented breaking change: older releases returned Maps fromgetNodeProps(), newer ones return plain objects. Ifedge.propsundergoes a similar evolution and returns a Map in a future git-warp version, the current code will silently fail.For consistency and robustness, wrap the edge properties iteration with the same abstraction:
Suggested fix pattern
- for (const [key, value] of Object.entries(edge.props)) { + for (const [key, value] of getPropEntries(edge.props)) {Same pattern applies to
src/merge.js:108.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/export.js` around lines 88 - 89, The iteration over edge properties uses Object.entries(edge.props) which bypasses the getPropEntries abstraction and may break if edge.props can be a Map; update the loop in export.js (the block that checks "if (edge.props) { for (const [key, value] of Object.entries(edge.props))") to use getPropEntries(edge.props) instead, mirroring how node properties are handled elsewhere, and apply the same replacement in merge.js at the loop around the code referenced on line 108 so both edge property iterations consistently use getPropEntries.src/merge.js (1)
107-113:⚠️ Potential issue | 🟡 MinorNormalize edge property iteration through
getPropEntries()for consistency.Edge properties on lines 108 and 89 (export.js) use raw
Object.entries(edge.props), while node properties usegetPropEntries(), which safely handles both Map and object prop formats. Although edge.props currently returns plain objects, git-warp's API contract shows edge props as Maps (matching node props), creating inconsistency that could break if the data structure changes.Either document that edge props are contractually always plain objects, or apply the same defensive pattern: replace
Object.entries(edge.props)withgetPropEntries(edge.props)in merge.js (line 108) and export.js (line 89) to match node property handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/merge.js` around lines 107 - 113, The loop over edge.props uses Object.entries(edge.props) which is inconsistent with node prop handling; replace Object.entries(edge.props) with getPropEntries(edge.props) in merge.js (the block that checks MERGE_EDGE_PROPS and calls patch.setEdgeProperty) and the similar spot in export.js to safely support Map or object prop formats; also ensure getPropEntries is imported/available in those modules and keep the existing checks for MERGE_EDGE_PROPS and value !== undefined/null before calling patch.setEdgeProperty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@GUIDE.md`:
- Line 67: GUIDE.md currently states git-mind "depends on the published
`@git-stunts/git-warp` package" which contradicts AGENTS.md and CLAUDE.md that
require "git-stunts/git-warp (local path)" and also mandates
"git-stunts/plumbing" as a direct dependency; pick one model and make
consistent: either revert GUIDE.md to state the local-path requirement (update
the `git-stunts/git-warp` phrasing) and ensure CLAUDE.md/AGENTS.md remain
accurate, or update AGENTS.md and CLAUDE.md to reflect the published
`@git-stunts/git-warp` model; finally, ensure package.json lists the chosen
artifacts correctly (add or change entries for "git-stunts/git-warp" and
"git-stunts/plumbing" in dependencies or local path entries) so all three
documents (GUIDE.md, AGENTS.md, CLAUDE.md) and package.json are consistent.
In `@package.json`:
- Line 29: Replace the semver entry for "@git-stunts/git-warp" in package.json
with a local file: dependency that points at your local git-stunts monorepo
build (i.e. the built/git-warp package in your monorepo) so the project uses the
local CRDT engine instead of an npm version; update the lockfile by running your
package manager install afterwards and verify the package resolves to the local
path (check package.json's "@git-stunts/git-warp" value and the resolved entry
in package-lock.json/yarn.lock).
In `@src/cli/commands.js`:
- Around line 94-99: The code sets config.match = getProp(propsMap, 'match')
without checking for undefined, which can cause graph.observer(observer, config)
to fail; update the logic around getProp, propsMap and graph.observer to
validate that match is present (e.g., call getProp(propsMap, 'match'), check if
the returned value is undefined or empty, and if so throw or return a clear
error message before calling graph.observer), leaving the existing guards for
optional expose/redact in place; reference getProp, propsMap, config and
graph.observer when making the change so the required match property is
validated and the code fails fast with a descriptive error instead of passing
undefined into graph.observer.
In `@src/content.js`:
- Around line 21-30: The decodeContent function currently always calls
Buffer.from(content) which copies when content is already a Buffer; update
decodeContent to first handle Buffer inputs by returning
content.toString('utf-8') directly, keep the existing branch for string inputs,
and only call Buffer.from(content).toString('utf-8') for Uint8Array/other binary
types; reference the decodeContent function to locate and change the code
accordingly.
In `@src/epoch.js`:
- Around line 73-77: The EpochInfo factory currently returns properties from
getProp that may be undefined; update the function that builds the EpochInfo
(the return containing tick, fullSha, recordedAt) to validate required fields
(at minimum tick, and fullSha if used downstream) and fail fast: if any required
property is missing, either throw a descriptive Error or return null/undefined
so callers like getEpochForRef can handle absence before calling
graph.materialize({ ceiling }); reference getProp, the returned EpochInfo, and
callers like getEpochForRef/graph.materialize when adding this defensive check.
In `@src/prop-bag.js`:
- Around line 20-26: getProp currently duck-types by checking for a .get method
which misclassifies objects with custom get functions; change the detection to
explicitly check for Map instances (use "props instanceof Map") in the getProp
function so Map-like behavior is only used for real Map objects, and fall back
to own-property lookup via Object.prototype.hasOwnProperty.call(props, key)
otherwise; update the branch that now returns props.get(key) to only run when
props is an instance of Map.
- Around line 34-40: The getPropEntries helper currently duck-types on
props.entries(), which is fragile; change the branch to check for a Map instance
instead: in the getPropEntries function, replace the typeof props.entries ===
'function' check with props instanceof Map and keep returning
[...props.entries()] for Map inputs, otherwise fall back to
Object.entries(props) (and still return [] when props is falsy).
- Around line 48-54: getPropSize currently treats any numeric props.size as the
count, which misfires for plain objects that legitimately have a numeric "size"
field; change getPropSize to only treat a numeric size as authoritative for
non-plain-object collections (e.g. Map/Set/Array) and fall back to
Object.keys(props).length for plain objects. Locate the getPropSize function and
add a plain-object check (e.g. Object.prototype.toString.call(props) ===
'[object Object]') and only return props.size when props is not a plain object
(or when props is an Array/Map/Set), otherwise return Object.keys(props).length.
In `@src/review.js`:
- Around line 106-111: The getReviewHistory path constructs a decision object
from propsMap without verifying source/target/edgeType first; mirror the
defensive check used in getPendingSuggestions by ensuring in getReviewHistory
you retrieve source = getProp(propsMap,'source'), target =
getProp(propsMap,'target'), edgeType = getProp(propsMap,'edgeType') and only
build/add the decision (and add to reviewedKeys) when source && target &&
edgeType are truthy, keeping the same `${source}|${target}|${edgeType}` key
construction and avoiding use of incomplete values.
- Around line 273-286: The code is pushing objects with potentially undefined
fields into decisions (see getProp usage and decisions.push), violating the
ReviewDecision typedef; update the logic in src/review.js to validate required
props (at minimum action and any string fields like source, target, reviewer)
before pushing: ensure action is one of 'accept'|'reject'|'adjust'|'skip' and
that required string props are non-empty, and if a node is malformed either skip
it (with a debug/error log) or throw a clear error; reference getProp,
decisions.push and the ReviewDecision shape when implementing the checks so
malformed prop-bags are filtered out before insertion.
In `@test/contracts.integration.test.js`:
- Line 135: The test suite applies SLOW_CLI_TIMEOUT to only 6 of 16 tests
inconsistently (e.g., the "nodes --id" test uses SLOW_CLI_TIMEOUT while "nodes
--json" does not); update the tests so CLI-invoking cases are handled
consistently by either adding the SLOW_CLI_TIMEOUT override to every test that
runs the CLI (search for tests named like "nodes --id" and "nodes --json") or
refactor to use a single helper (e.g., runCliTest or a wrapper around it) that
always applies SLOW_CLI_TIMEOUT to avoid ad-hoc per-test timeouts, and if some
tests genuinely need longer timeouts add brief comments documenting why.
- Line 18: SLOW_CLI_TIMEOUT (15_000) is shorter than the timeout used by runCli
(timeout: 30_000), causing flakiness; fix by making the test harness timeout >=
the CLI exec timeout or lowering the CLI exec timeout to match: either set
SLOW_CLI_TIMEOUT to 30_000 (or higher) or change the timeout passed in
runCli/execFileSync to 15_000 so both timeouts align (update the constants and
the timeout option in runCli accordingly).
In `@test/prop-bag.test.js`:
- Around line 4-36: Add unit tests exercising edge cases for the prop-bag
helpers: call getProp, getPropSize, getPropEntries and toPropObject with null
and undefined and assert they return safe defaults (e.g., undefined for getProp,
0 for getPropSize, empty array for getPropEntries, and empty object for
toPropObject); add tests that getProp returns undefined for non-existent keys;
add a prototype-pollution test by creating an object with inherited properties
(Object.create({ inherited: 'bad' })) and asserting getProp(obj, 'inherited') is
undefined while getProp(obj, 'own') returns the own property; and assert
getPropSize(new Map()) and getPropSize({}) return 0 so getProp, getPropSize,
getPropEntries, and toPropObject are validated against these edge cases.
---
Outside diff comments:
In `@src/export.js`:
- Around line 88-89: The iteration over edge properties uses
Object.entries(edge.props) which bypasses the getPropEntries abstraction and may
break if edge.props can be a Map; update the loop in export.js (the block that
checks "if (edge.props) { for (const [key, value] of
Object.entries(edge.props))") to use getPropEntries(edge.props) instead,
mirroring how node properties are handled elsewhere, and apply the same
replacement in merge.js at the loop around the code referenced on line 108 so
both edge property iterations consistently use getPropEntries.
In `@src/merge.js`:
- Around line 107-113: The loop over edge.props uses Object.entries(edge.props)
which is inconsistent with node prop handling; replace
Object.entries(edge.props) with getPropEntries(edge.props) in merge.js (the
block that checks MERGE_EDGE_PROPS and calls patch.setEdgeProperty) and the
similar spot in export.js to safely support Map or object prop formats; also
ensure getPropEntries is imported/available in those modules and keep the
existing checks for MERGE_EDGE_PROPS and value !== undefined/null before calling
patch.setEdgeProperty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 743ef6a8-c1cc-4808-b5b6-3f2bcb3462c5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
AGENTS.mdCLAUDE.mdCONTRIBUTING.mdGUIDE.mdROADMAP.mdTECH-PLAN.mddocs/README.mddocs/design/git-warp-upgrade-audit.mdpackage.jsonsrc/cli/commands.jssrc/content.jssrc/epoch.jssrc/export.jssrc/merge.jssrc/nodes.jssrc/prop-bag.jssrc/review.jssrc/suggest.jssrc/views.jstest/contracts.integration.test.jstest/diff.test.jstest/graph.test.jstest/import.test.jstest/merge.test.jstest/prop-bag.test.js
| ``` | ||
|
|
||
| git-mind depends on `@git-stunts/git-warp`, which is currently installed from a local path. See [CONTRIBUTING.md](CONTRIBUTING.md) for development setup details. | ||
| git-mind depends on the published `@git-stunts/git-warp` package. See [CONTRIBUTING.md](CONTRIBUTING.md) for current development setup details. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify CONTRIBUTING.md mentions the published package model
echo "=== Checking CONTRIBUTING.md for git-warp setup guidance ==="
rg -n -C3 "git-warp|git-stunts" CONTRIBUTING.md | head -30Repository: flyingrobots/git-mind
Length of output: 307
BLOCKING: GUIDE.md contradicts documented dependency model.
Line 67 claims git-mind "depends on the published @git-stunts/git-warp package," but AGENTS.md and CLAUDE.md explicitly require "git-stunts/git-warp (local path)" as the core dependency for CRDT graph on Git. This mismatch must be resolved before merging. Either:
- The documentation is stale and git-warp was never published (revert GUIDE.md to clarify local-path requirement), or
- AGENTS.md and CLAUDE.md require updating to reflect the published package model.
Additionally, CLAUDE.md explicitly mandates "git-stunts/plumbing is installed as a direct dependency"—ensure package.json reflects both requirements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@GUIDE.md` at line 67, GUIDE.md currently states git-mind "depends on the
published `@git-stunts/git-warp` package" which contradicts AGENTS.md and
CLAUDE.md that require "git-stunts/git-warp (local path)" and also mandates
"git-stunts/plumbing" as a direct dependency; pick one model and make
consistent: either revert GUIDE.md to state the local-path requirement (update
the `git-stunts/git-warp` phrasing) and ensure CLAUDE.md/AGENTS.md remain
accurate, or update AGENTS.md and CLAUDE.md to reflect the published
`@git-stunts/git-warp` model; finally, ensure package.json lists the chosen
artifacts correctly (add or change entries for "git-stunts/git-warp" and
"git-stunts/plumbing" in dependencies or local path entries) so all three
documents (GUIDE.md, AGENTS.md, CLAUDE.md) and package.json are consistent.
| }, | ||
| "dependencies": { | ||
| "@git-stunts/git-warp": "^11.5.0", | ||
| "@git-stunts/git-warp": "^14.16.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify `@git-stunts/git-warp` v14.16.2 is referenced correctly in package-lock.json
# and check for any version mismatches after npm install
if [ -f "package-lock.json" ]; then
echo "=== Checking package-lock.json for git-warp version ==="
cat package-lock.json | jq '.packages["node_modules/@git-stunts/git-warp"].version // .dependencies["@git-stunts/git-warp"].version' 2>/dev/null || echo "Could not parse package-lock.json"
fi
echo ""
echo "=== Searching for git-warp API usage patterns in src/ ==="
rg -n "getNodeProps|createPatch|addNode|addEdge|setProperty|attachContent|discoverTicks|materialize" --type js src/ | head -30Repository: flyingrobots/git-mind
Length of output: 2347
🏁 Script executed:
# Check prop-bag.js to see how it handles the breaking change
cat -n src/prop-bag.js | head -30Repository: flyingrobots/git-mind
Length of output: 1014
🏁 Script executed:
# Check if git-stunts/git-warp exists as a local dependency in the repo
find . -type d -name "git-warp" -o -name "git-stunts" | head -20Repository: flyingrobots/git-mind
Length of output: 47
🏁 Script executed:
# Check the actual package.json to see the current dependencies around git-warp
cat -n package.json | sed -n '25,50p'Repository: flyingrobots/git-mind
Length of output: 757
Use local file path for @git-stunts/git-warp, not semver version — violates coding guidelines.
Replace "@git-stunts/git-warp": "^14.16.2" with a local path reference per guidelines. The codebase depends on git-warp as the core CRDT graph engine and must pin to your local git-stunts monorepo build, not npm registry versions. The prop-bag.js shim correctly handles the v14 breaking change (Map → plain object in getNodeProps), so API compatibility is not a blocker—but the dependency specification itself is non-compliant.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 29, Replace the semver entry for "@git-stunts/git-warp"
in package.json with a local file: dependency that points at your local
git-stunts monorepo build (i.e. the built/git-warp package in your monorepo) so
the project uses the local CRDT engine instead of an npm version; update the
lockfile by running your package manager install afterwards and verify the
package resolves to the local path (check package.json's "@git-stunts/git-warp"
value and the resolved entry in package-lock.json/yarn.lock).
|
@coderabbitai Thanks. Two dependency-model findings on this PR are stale after the governance/docs alignment that already landed on : the published-package model is intentional, and the current branch already reflects it consistently in , , , , and . The actionable feedback on the runtime boundary has been fixed in the latest commits; the local-path recommendation should be treated as a mix-up, not as the target state for this repo. |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
⛔ Learnings removed🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/epoch.js`:
- Around line 77-83: The current validation in the epoch record block only
checks typeof tick === 'number' which allows NaN, Infinity, and negatives;
update the check around the existing if ( ... ) so tick is further validated
with Number.isFinite(tick) and Number.isInteger(tick) (or Math.floor check) and
tick >= 0 to reject NaN/Infinity/non-integers/negative values, while keeping the
existing fullSha and recordedAt string/length checks; ensure the conditional
uses the same variable names (tick, fullSha, recordedAt) so malformed epoch.tick
values are rejected.
In `@src/review.js`:
- Around line 290-291: Update the numeric validation in the decision-node
validation (replace the current typeof checks for confidence/timestamp) to
reject NaN and infinities by using Number.isFinite(confidence) and
Number.isFinite(timestamp) (or invert with !Number.isFinite) instead of typeof,
e.g., change the condition that currently uses "typeof confidence !== 'number'
|| typeof timestamp !== 'number'" to use Number.isFinite checks so NaN and
+/-Infinity are treated as invalid in the validation logic in src/review.js.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 48feedf7-480f-4a2e-85f0-638483f85145
📒 Files selected for processing (14)
CHANGELOG.mdsrc/cli/commands.jssrc/content.jssrc/epoch.jssrc/export.jssrc/merge.jssrc/prop-bag.jssrc/review.jstest/contracts.integration.test.jstest/epoch.test.jstest/export.test.jstest/prop-bag.test.jstest/resolve-context.test.jstest/review.test.js
| if ( | ||
| typeof tick !== 'number' || | ||
| typeof fullSha !== 'string' || | ||
| fullSha.length === 0 || | ||
| typeof recordedAt !== 'string' || | ||
| recordedAt.length === 0 | ||
| ) { |
There was a problem hiding this comment.
tick validation is too weak for malformed records.
Line 78 only checks typeof tick === 'number', so NaN, Infinity, and negative values pass. That can leak invalid epoch.tick into downstream materialization.
Suggested fix
if (
- typeof tick !== 'number' ||
+ !Number.isInteger(tick) ||
+ tick < 0 ||
typeof fullSha !== 'string' ||
fullSha.length === 0 ||
typeof recordedAt !== 'string' ||
recordedAt.length === 0
) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| typeof tick !== 'number' || | |
| typeof fullSha !== 'string' || | |
| fullSha.length === 0 || | |
| typeof recordedAt !== 'string' || | |
| recordedAt.length === 0 | |
| ) { | |
| if ( | |
| !Number.isInteger(tick) || | |
| tick < 0 || | |
| typeof fullSha !== 'string' || | |
| fullSha.length === 0 || | |
| typeof recordedAt !== 'string' || | |
| recordedAt.length === 0 | |
| ) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/epoch.js` around lines 77 - 83, The current validation in the epoch
record block only checks typeof tick === 'number' which allows NaN, Infinity,
and negatives; update the check around the existing if ( ... ) so tick is
further validated with Number.isFinite(tick) and Number.isInteger(tick) (or
Math.floor check) and tick >= 0 to reject NaN/Infinity/non-integers/negative
values, while keeping the existing fullSha and recordedAt string/length checks;
ensure the conditional uses the same variable names (tick, fullSha, recordedAt)
so malformed epoch.tick values are rejected.
| typeof confidence !== 'number' || | ||
| typeof timestamp !== 'number' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Pedantic nitpick: NaN slips through numeric validation.
typeof NaN === 'number' evaluates to true, so a decision node with confidence: NaN or timestamp: NaN would pass validation and pollute downstream consumers.
Consider Number.isFinite() for paranoid-level defense:
- typeof confidence !== 'number' ||
- typeof timestamp !== 'number'
+ !Number.isFinite(confidence) ||
+ !Number.isFinite(timestamp)Realistically, this requires deliberate sabotage of graph data. Up to you whether the extra paranoia is warranted.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| typeof confidence !== 'number' || | |
| typeof timestamp !== 'number' | |
| !Number.isFinite(confidence) || | |
| !Number.isFinite(timestamp) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/review.js` around lines 290 - 291, Update the numeric validation in the
decision-node validation (replace the current typeof checks for
confidence/timestamp) to reject NaN and infinities by using
Number.isFinite(confidence) and Number.isFinite(timestamp) (or invert with
!Number.isFinite) instead of typeof, e.g., change the condition that currently
uses "typeof confidence !== 'number' || typeof timestamp !== 'number'" to use
Number.isFinite checks so NaN and +/-Infinity are treated as invalid in the
validation logic in src/review.js.
1 participant
PR Title
feat: upgrade git-warp to v14.16.2
Summary
@git-stunts/git-warpfrom11.5.0to14.16.2Problem Statement
@git-stunts/git-warpand was still coded against the older Map-shapedgetNodeProps()behavior.ADR Compliance (Required)
Relevant ADR(s)
Compliance Declaration
Exception Request (Required if deviating)
Architecture Laws Checklist (Hard Gates)
Canonical Truth & Context
--at,--observer,--trust) or deterministically defaulted.Determinism & Provenance
Artifact Hygiene
Contracts & Compatibility
Extension/Effects Safety (if applicable)
Scope Control
Backward Compatibility
Test Plan (Required)
Unit
npm test -- test/prop-bag.test.js test/resolve-context.test.js test/review.test.js test/epoch.test.js test/export.test.js test/contracts.integration.test.jsIntegration
npm test -- test/contracts.integration.test.jsDeterminism
npm test -- test/diff.test.js test/graph.test.js test/import.test.js test/merge.test.jsContract/Schema
npm test -- test/contracts.integration.test.js test/contracts.test.jsPolicy Gates
npm run lint npm testSecurity / Trust Impact
matchnow fail fast with an explicit error.Performance Impact
Observability / Debuggability
Operational Notes
Linked Issues / Milestones
Reviewer Quick Verdict Block (for maintainers)
MUST (Hard Gates)
SHOULD (Quality)
Verdict