fix(execution): null guard em getLastCheckpoint/getCheckpoint (#513)#523
fix(execution): null guard em getLastCheckpoint/getCheckpoint (#513)#523nikolasdehor wants to merge 2 commits intoSynkraAI:mainfrom
Conversation
getLastCheckpoint() acessava this._state.checkpoints.length sem verificar se checkpoints existe. State carregado de JSON incompleto (sem a propriedade checkpoints) causa TypeError em runtime. Adiciona guard !this._state.checkpoints em ambos os métodos. Closes SynkraAI#513
|
@nikolasdehor is attempting to deploy a commit to the Pedro Valério Lopez's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdded defensive null/undefined checks in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
This pull request addresses a defensive programming issue in the BuildStateManager where accessing this._state.checkpoints without first verifying it exists could cause a TypeError. The fix adds null guards to getLastCheckpoint() and getCheckpoint() methods to prevent crashes when the state's checkpoints property is undefined.
Changes:
- Added null guard for
this._state.checkpointsingetLastCheckpoint()method - Added null guard for
this._state.checkpointsingetCheckpoint()method
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.aios-core/core/execution/build-state-manager.js (1)
365-366: Consider adding a similar null guard here for consistency.While
saveCheckpointalready validates!this._state, the same corrupted-state scenario (state exists butcheckpointsis undefined) could cause a TypeError at line 366. For defense-in-depth, consider initializingcheckpointsif missing:🛡️ Optional defensive initialization
// Add to state + if (!this._state.checkpoints) { + this._state.checkpoints = []; + } this._state.checkpoints.push(checkpoint);Alternatively, throw a more descriptive error if state integrity is compromised. This is optional and can be addressed separately if needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aios-core/core/execution/build-state-manager.js around lines 365 - 366, The push to this._state.checkpoints can throw if this._state exists but checkpoints is undefined; in the saveCheckpoint method (or wherever this._state.checkpoints.push(checkpoint) is called) add a defensive guard that ensures this._state.checkpoints is an array before pushing (e.g. if (!Array.isArray(this._state.checkpoints)) this._state.checkpoints = [];), or alternatively throw a clear error indicating corrupted state; update the code around the this._state.checkpoints.push(checkpoint) call to perform that check/initialization to avoid a TypeError.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.aios-core/core/execution/build-state-manager.js:
- Around line 365-366: The push to this._state.checkpoints can throw if
this._state exists but checkpoints is undefined; in the saveCheckpoint method
(or wherever this._state.checkpoints.push(checkpoint) is called) add a defensive
guard that ensures this._state.checkpoints is an array before pushing (e.g. if
(!Array.isArray(this._state.checkpoints)) this._state.checkpoints = [];), or
alternatively throw a clear error indicating corrupted state; update the code
around the this._state.checkpoints.push(checkpoint) call to perform that
check/initialization to avoid a TypeError.
Problema
Em
build-state-manager.js:400,getLastCheckpoint()acessathis._state.checkpoints.lengthdiretamente. Sethis._stateexiste mascheckpointséundefined(state carregado de JSON incompleto), ocorre TypeError: Cannot read property 'length' of undefined.Mesmo bug em
getCheckpoint()(linha 416): acessa.checkpoints.find()sem guard.Correção
getLastCheckpoint() { - if (!this._state || this._state.checkpoints.length === 0) { + if (!this._state || !this._state.checkpoints || this._state.checkpoints.length === 0) { return null; } getCheckpoint(checkpointId) { - if (!this._state) { + if (!this._state || !this._state.checkpoints) { return null; }Validação
Closes #513
Summary by CodeRabbit
Bug Fixes
Chores