feat/odedgo/atp-checkpointer #54
Conversation
| /** | ||
| * Default configuration values | ||
| */ | ||
| export const DEFAULT_CHECKPOINT_CONFIG: Required<Omit<CheckpointConfig, 'strategy'>> = { |
There was a problem hiding this comment.
Move from types to consts file
| lines.push('In your next code iteration, you can:'); | ||
| lines.push('1. Directly use data returned from saved checkpoints (full_snapshot)'); | ||
| lines.push('2. Restore a checkpoint value programmatically using:'); | ||
| lines.push(' const value = await __restore.checkpoint("<checkpoint_id>");'); |
There was a problem hiding this comment.
Maybe define '__restore' and 'checkpoint' as consts, and reuse them when adding it to the runtime global at packages/server/src/executor/sandbox-injector.ts
There was a problem hiding this comment.
Tho its up to the LLM to choose from, ur suggestion always "injecting" them ?
There was a problem hiding this comment.
Just ment that '__restore' and 'checkpoint' won't be hardcoded in the prompt, since it will break stuff.
You can do something like
const RESTORE_OPERATION = '__restore';
lines.push(`...${RESTORE_OPERATIPN}`)
and in sandbox-injector.js
checkpointSetup += `
globalThis.${RESTORE_OPERATION} = {
...
}
};`;
( same for checkpointer )
| console.log('\n[TEST] Verified: Sensitive data not exposed in checkpoint'); | ||
| }); | ||
|
|
||
| test('should block if LLM tries to use checkpoint data directly (without restore)', async () => { |
There was a problem hiding this comment.
Not sure I get this test - It doesn't test anything. Regardless, I would remove it
There was a problem hiding this comment.
Me neither 😓 removing
| return { user, orders, result }; | ||
| `; | ||
|
|
||
| const executeResponse = await fetch(`${BASE_URL}/api/execute`, { |
There was a problem hiding this comment.
Wouldn't it be simpler to create atp client and use client.execute rather then implementing the API calls?
There was a problem hiding this comment.
MAybe, i just followed existing conventions
Main changes