Skip to content

fix(core): fall back to empty on malformed agents.json shape #29208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
soroush5 wants to merge 2 commits into
base: main
Choose a base branch
from
+56 −2
Open

fix(core): fall back to empty on malformed agents.json shape #29208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6594dcc
fix(core): fall back to empty on malformed agents.json shape
soroush5 Sep 4, 2026
b8cd71b
fix(core): reject non-map project entries in acknowledged-agents guard
soroush5 Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions packages/core/src/agents/acknowledgedAgents.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,34 @@ describe('AcknowledgedAgentsService', () => {
false,
);
});

it.each([
'null',
'42',
'"str"',
'[]',
'{"/project": "str"}',
'{"/project": 42}',
'{"/project": null}',
'{"/project": []}',
])(
'should fall back to empty for valid JSON with the wrong shape (%s, #29207)',
async (content) => {
const ackPath = Storage.getAcknowledgedAgentsPath();
await fs.mkdir(path.dirname(ackPath), { recursive: true });
await fs.writeFile(ackPath, content, 'utf-8');

const service = new AcknowledgedAgentsService();

await expect(
service.isAcknowledged('/project', 'Agent', 'hash'),
).resolves.toBe(false);
await expect(
service.acknowledge('/project', 'Agent', 'hash'),
).resolves.toBeUndefined();
await expect(
service.isAcknowledged('/project', 'Agent', 'hash'),
).resolves.toBe(true);
},
);
});
28 changes: 26 additions & 2 deletions packages/core/src/agents/acknowledgedAgents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,20 @@ export interface AcknowledgedAgentsMap {
};
}

function isAcknowledgedAgentsMap(
value: unknown,
): value is AcknowledgedAgentsMap {
if (!value || typeof value !== 'object' || Array.isArray(value)) {
return false;
}
// Each project entry must itself be a map object: a truthy primitive
// (e.g. {"/project": "str"}) would otherwise pass this guard and then
// crash acknowledge() with "Cannot create property on string".
return Object.values(value).every(
(entry) => !!entry && typeof entry === 'object' && !Array.isArray(entry),
);
}
Comment on lines +20 to +32

@gemini-code-assist gemini-code-assist Bot Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The type guard isAcknowledgedAgentsMap only performs a shallow check on the top-level value. If the JSON is an object but contains non-object values (e.g., {"/project": "string"}), isAcknowledgedAgentsMap will return true. This will cause acknowledge to crash with a TypeError when trying to set a property on a primitive string (e.g., TypeError: Cannot create property 'Agent' on string).

To prevent this, the type guard should also verify that all values within the map are non-null, non-array objects.

function isAcknowledgedAgentsMap(
  value: unknown,
): value is AcknowledgedAgentsMap {
  if (!value || typeof value !== 'object' || Array.isArray(value)) {
    return false;
  }
  return Object.values(value).every(
    (val) => !!val && typeof val === 'object' && !Array.isArray(val),
  );
}

@soroush5 soroush5 Sep 4, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — I reproduced it: {"/project": "str"} passes the old guard and then acknowledge() throws TypeError: Cannot create property 'Agent' on string. Fixed by requiring every project entry to be a non-null, non-array object (falls back to empty otherwise), plus regression tests for the nested shapes. Pushed in b8cd71b — 12/12 green.


export class AcknowledgedAgentsService {
private acknowledgedAgents: AcknowledgedAgentsMap = {};
private loaded = false;
Expand All @@ -27,8 +41,18 @@ export class AcknowledgedAgentsService {
const filePath = Storage.getAcknowledgedAgentsPath();
try {
const content = await fs.readFile(filePath, 'utf-8');
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
this.acknowledgedAgents = JSON.parse(content);
const parsed: unknown = JSON.parse(content);
// A previous interrupted save (full disk, sync conflict, hand edit)
// can leave valid JSON with the wrong shape (null, array, scalar).
// Fall back to empty rather than crashing callers (#29207).
if (isAcknowledgedAgentsMap(parsed)) {
this.acknowledgedAgents = parsed;
} else {
debugLogger.error(
'Failed to load acknowledged agents: unexpected file shape, falling back to empty.',
);
this.acknowledgedAgents = {};
}
} catch (error: unknown) {
if (!isNodeError(error) || error.code !== 'ENOENT') {
debugLogger.error(
Expand Down
Loading