Skip to content

Commit 5543425

Browse files
committed
fix: address review findings from code review
- Fix merge_menu KeyError crash when menu items missing 'code' key - Fix _is_menu_array to check ALL elements, not just first - Remove unused import os from resolve-customization.py - Remove inject.after from agent activation and customize.toml
1 parent be530e3 commit 5543425

49 files changed

Lines changed: 210 additions & 196 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/agents/gds-agent-game-architect/SKILL.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ Use the JSON output as resolved values.
2424
incorporate its content as high-priority context.
2525
3. **Load resources** -- If `additional_resources` is not empty, read
2626
each listed file and incorporate as reference context.
27-
4. **Inject after** -- If `inject.after` is not empty, read and
28-
incorporate its content as supplementary context.
2927

3028
You must fully embody this persona so the user gets the best experience and help they need. Do not break character until the user dismisses this persona. When the user calls a skill, this persona must carry through and remain active.
3129

src/agents/gds-agent-game-architect/customize.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,3 @@ Architecture is about delaying decisions until you have enough data. Build for t
5353
# ──────────────────────────────────────────────────────────────────
5454
[inject]
5555
before = ""
56-
after = ""

src/agents/gds-agent-game-architect/scripts/resolve-customization.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
import argparse
2323
import json
24-
import os
2524
import sys
2625
import tomllib
2726
from pathlib import Path
@@ -57,19 +56,21 @@ def load_toml(path: Path) -> dict[str, Any]:
5756
# ---------------------------------------------------------------------------
5857

5958
def _is_menu_array(value: Any) -> bool:
60-
"""True when *value* looks like a ``[[menu]]`` array of tables with ``code`` keys."""
59+
"""True when *value* is a non-empty list where ALL items are dicts with a ``code`` key."""
6160
return (
6261
isinstance(value, list)
6362
and len(value) > 0
64-
and isinstance(value[0], dict)
65-
and "code" in value[0]
63+
and all(isinstance(item, dict) and "code" in item for item in value)
6664
)
6765

6866

6967
def merge_menu(base: list[dict], override: list[dict]) -> list[dict]:
7068
"""Merge-by-code: matching codes replace; new codes append."""
71-
result_by_code: dict[str, dict] = {item["code"]: dict(item) for item in base}
69+
result_by_code: dict[str, dict] = {item["code"]: dict(item) for item in base if "code" in item}
7270
for item in override:
71+
if "code" not in item:
72+
print(f"warning: menu item missing 'code' key, skipping: {item}", file=sys.stderr)
73+
continue
7374
result_by_code[item["code"]] = dict(item)
7475
return list(result_by_code.values())
7576

src/agents/gds-agent-game-designer/SKILL.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ Use the JSON output as resolved values.
2424
incorporate its content as high-priority context.
2525
3. **Load resources** -- If `additional_resources` is not empty, read
2626
each listed file and incorporate as reference context.
27-
4. **Inject after** -- If `inject.after` is not empty, read and
28-
incorporate its content as supplementary context.
2927

3028
You must fully embody this persona so the user gets the best experience and help they need. Do not break character until the user dismisses this persona. When the user calls a skill, this persona must carry through and remain active.
3129

src/agents/gds-agent-game-designer/customize.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,3 @@ Design what players want to FEEL, not what they say they want. Prototype fast -
5353
# ──────────────────────────────────────────────────────────────────
5454
[inject]
5555
before = ""
56-
after = ""

src/agents/gds-agent-game-designer/scripts/resolve-customization.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
import argparse
2323
import json
24-
import os
2524
import sys
2625
import tomllib
2726
from pathlib import Path
@@ -57,19 +56,21 @@ def load_toml(path: Path) -> dict[str, Any]:
5756
# ---------------------------------------------------------------------------
5857

5958
def _is_menu_array(value: Any) -> bool:
60-
"""True when *value* looks like a ``[[menu]]`` array of tables with ``code`` keys."""
59+
"""True when *value* is a non-empty list where ALL items are dicts with a ``code`` key."""
6160
return (
6261
isinstance(value, list)
6362
and len(value) > 0
64-
and isinstance(value[0], dict)
65-
and "code" in value[0]
63+
and all(isinstance(item, dict) and "code" in item for item in value)
6664
)
6765

6866

6967
def merge_menu(base: list[dict], override: list[dict]) -> list[dict]:
7068
"""Merge-by-code: matching codes replace; new codes append."""
71-
result_by_code: dict[str, dict] = {item["code"]: dict(item) for item in base}
69+
result_by_code: dict[str, dict] = {item["code"]: dict(item) for item in base if "code" in item}
7270
for item in override:
71+
if "code" not in item:
72+
print(f"warning: menu item missing 'code' key, skipping: {item}", file=sys.stderr)
73+
continue
7374
result_by_code[item["code"]] = dict(item)
7475
return list(result_by_code.values())
7576

src/agents/gds-agent-game-dev/SKILL.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ Use the JSON output as resolved values.
2424
incorporate its content as high-priority context.
2525
3. **Load resources** -- If `additional_resources` is not empty, read
2626
each listed file and incorporate as reference context.
27-
4. **Inject after** -- If `inject.after` is not empty, read and
28-
incorporate its content as supplementary context.
2927

3028
You must fully embody this persona so the user gets the best experience and help they need. Do not break character until the user dismisses this persona. When the user calls a skill, this persona must carry through and remain active.
3129

src/agents/gds-agent-game-dev/customize.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,3 @@ principles = """\
5353
# ──────────────────────────────────────────────────────────────────
5454
[inject]
5555
before = ""
56-
after = ""

src/agents/gds-agent-game-dev/scripts/resolve-customization.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
import argparse
2323
import json
24-
import os
2524
import sys
2625
import tomllib
2726
from pathlib import Path
@@ -57,19 +56,21 @@ def load_toml(path: Path) -> dict[str, Any]:
5756
# ---------------------------------------------------------------------------
5857

5958
def _is_menu_array(value: Any) -> bool:
60-
"""True when *value* looks like a ``[[menu]]`` array of tables with ``code`` keys."""
59+
"""True when *value* is a non-empty list where ALL items are dicts with a ``code`` key."""
6160
return (
6261
isinstance(value, list)
6362
and len(value) > 0
64-
and isinstance(value[0], dict)
65-
and "code" in value[0]
63+
and all(isinstance(item, dict) and "code" in item for item in value)
6664
)
6765

6866

6967
def merge_menu(base: list[dict], override: list[dict]) -> list[dict]:
7068
"""Merge-by-code: matching codes replace; new codes append."""
71-
result_by_code: dict[str, dict] = {item["code"]: dict(item) for item in base}
69+
result_by_code: dict[str, dict] = {item["code"]: dict(item) for item in base if "code" in item}
7270
for item in override:
71+
if "code" not in item:
72+
print(f"warning: menu item missing 'code' key, skipping: {item}", file=sys.stderr)
73+
continue
7374
result_by_code[item["code"]] = dict(item)
7475
return list(result_by_code.values())
7576

src/agents/gds-agent-game-qa/SKILL.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ Use the JSON output as resolved values.
2424
incorporate its content as high-priority context.
2525
3. **Load resources** -- If `additional_resources` is not empty, read
2626
each listed file and incorporate as reference context.
27-
4. **Inject after** -- If `inject.after` is not empty, read and
28-
incorporate its content as supplementary context.
2927

3028
You must fully embody this persona so the user gets the best experience and help they need. Do not break character until the user dismisses this persona. When the user calls a skill, this persona must carry through and remain active.
3129

0 commit comments

Comments
 (0)