feat: add DNCDPRG.lst symbol table with MCP tools#87
Closed
Copilot wants to merge 8 commits into
Closed
Conversation
Agent-Logs-Url: https://github.com/OpenRakis/Cryogenic/sessions/ebdbdb95-7aa3-47ae-b6b2-f8f2c4b50d76 Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>
Agent-Logs-Url: https://github.com/OpenRakis/Cryogenic/sessions/ebdbdb95-7aa3-47ae-b6b2-f8f2c4b50d76 Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>
Agent-Logs-Url: https://github.com/OpenRakis/Cryogenic/sessions/ebdbdb95-7aa3-47ae-b6b2-f8f2c4b50d76 Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
maximilien-noal
May 7, 2026 05:10
View session
Agent-Logs-Url: https://github.com/OpenRakis/Cryogenic/sessions/daf9699d-2c53-4056-9887-5ae6e715fab4 Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>
maximilien-noal
requested changes
May 7, 2026
Member
maximilien-noal
left a comment
maximilien-noal
left a comment
There was a problem hiding this comment.
@copilot use arrange act assert for tests.
Use AwesomeAssertions.
Agent-Logs-Url: https://github.com/OpenRakis/Cryogenic/sessions/88620129-716f-47ab-9a3a-065a8829e7a8 Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>
Contributor
Author
Done in 919ca80. Added Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
maximilien-noal
requested changes
May 7, 2026
Member
maximilien-noal
left a comment
maximilien-noal
left a comment
There was a problem hiding this comment.
@copilot never use calculated properties.
Don't use lock for a frozen hash set !
Use injection of dependencies via constructors.
Make the entire class not static.
Do not use static properties.
Do not use nullables.
…es/lock Agent-Logs-Url: https://github.com/OpenRakis/Cryogenic/sessions/a093cd2e-0009-496d-9f3b-3d76408c6ec4 Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>
Contributor
Author
Done in adf734b.
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Agent-Logs-Url: https://github.com/OpenRakis/Cryogenic/sessions/a093cd2e-0009-496d-9f3b-3d76408c6ec4 Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>
Agent-Logs-Url: https://github.com/OpenRakis/Cryogenic/sessions/a093cd2e-0009-496d-9f3b-3d76408c6ec4 Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>
maximilien-noal
approved these changes
May 7, 2026
There was a problem hiding this comment.
Pull request overview
This PR introduces a parsed symbol table for the embedded DNCDPRG.lst listing and exposes it via MCP tools to support live reverse-engineering workflows in Cryogenic/Spice86.
Changes:
- Added a
Cryogenic.SymbolsSRP-style symbol parsing/lookup API plus an embeddedDNCDPRG.lstresource. - Added a non-static MCP tool class (
CryogenicSymbolMcpTools) for symbol lookup/search/list, backed by DI-providedILstSymbolTable. - Added an xUnit test project with AwesomeAssertions coverage for the parser/table/factory.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Cryogenic/Symbols/LstSymbolTableFactory.cs | Loads embedded .lst resource and builds a symbol table via parser |
| src/Cryogenic/Symbols/LstSymbolTable.cs | Implements address/name lookup over parsed symbols |
| src/Cryogenic/Symbols/LstSymbolParser.cs | Parses IDA-style listing label lines into LstSymbol entries |
| src/Cryogenic/Symbols/LstSymbolLookup.cs | Non-nullable lookup-result wrapper (Found + Symbol) |
| src/Cryogenic/Symbols/LstSymbol.cs | Symbol record model (segment/offset/name/autolabel) |
| src/Cryogenic/Symbols/ILstSymbolTable.cs | Public symbol table lookup/list API contract |
| src/Cryogenic/Symbols/ILstSymbolParser.cs | Parser interface for .lst lines |
| src/Cryogenic/DuneCdOverrideSupplier.cs | Registers ILstSymbolTable as an MCP service |
| src/Cryogenic/CryogenicSymbolMcpTools.cs | New MCP tools: lookup/search/list for .lst symbols |
| src/Cryogenic/CryogenicMcpTools.cs | Whitespace-only change at EOF |
| src/Cryogenic/Cryogenic.csproj | Embeds doc/DNCDPRG.lst as Cryogenic.DNCDPRG.lst |
| src/Cryogenic.Tests/Symbols/LstSymbolTableTests.cs | Unit tests for table lookup behaviors |
| src/Cryogenic.Tests/Symbols/LstSymbolTableFactoryTests.cs | Integration tests validating embedded resource loading |
| src/Cryogenic.Tests/Symbols/LstSymbolParserTests.cs | Unit tests for parser correctness and edge cases |
| src/Cryogenic.Tests/Cryogenic.Tests.csproj | New xUnit test project with AwesomeAssertions + tooling |
| src/Cryogenic.sln | Adds the new test project to the solution |
Comment on lines
+42
to
+47
| public ILstSymbolTable Build() { | ||
| Assembly assembly = typeof(LstSymbolTableFactory).Assembly; | ||
| using Stream? stream = assembly.GetManifestResourceStream(ResourceName); | ||
| if (stream is null) { | ||
| return new LstSymbolTable(new List<LstSymbol>()); | ||
| } |
Comment on lines
+55
to
+60
| private static IEnumerable<string> ReadLines(StreamReader reader) { | ||
| while (!reader.EndOfStream) { | ||
| string? line = reader.ReadLine(); | ||
| if (line is not null) { | ||
| yield return line; | ||
| } |
Comment on lines
+38
to
+41
| public LstSymbolLookup FindByAddress(string segment, ushort offset) { | ||
| if (_byAddress.TryGetValue((segment, offset), out LstSymbol? sym)) { | ||
| return new LstSymbolLookup { Found = true, Symbol = sym }; | ||
| } |
Comment on lines
+36
to
+47
| /// <returns>All matching symbols ordered by segment then offset.</returns> | ||
| IReadOnlyList<LstSymbol> FindByName(string nameSubstring, bool includeAutoLabels = false); | ||
|
|
||
| /// <summary> | ||
| /// Returns all symbols in the table. | ||
| /// </summary> | ||
| /// <param name="includeAutoLabels"> | ||
| /// When <c>true</c>, includes auto-generated labels (loc_, sub_). | ||
| /// Defaults to <c>false</c>. | ||
| /// </param> | ||
| /// <returns>All symbols ordered by segment then offset.</returns> | ||
| IReadOnlyList<LstSymbol> GetAll(bool includeAutoLabels = false); |
Comment on lines
+29
to
+47
| /// (case-insensitive, ordinal comparison). | ||
| /// </summary> | ||
| /// <param name="nameSubstring">Substring to match against symbol names.</param> | ||
| /// <param name="includeAutoLabels"> | ||
| /// When <c>true</c>, includes auto-generated labels whose names start with "loc_" or "sub_". | ||
| /// Defaults to <c>false</c> so only meaningful reverse-engineered names are returned. | ||
| /// </param> | ||
| /// <returns>All matching symbols ordered by segment then offset.</returns> | ||
| IReadOnlyList<LstSymbol> FindByName(string nameSubstring, bool includeAutoLabels = false); | ||
|
|
||
| /// <summary> | ||
| /// Returns all symbols in the table. | ||
| /// </summary> | ||
| /// <param name="includeAutoLabels"> | ||
| /// When <c>true</c>, includes auto-generated labels (loc_, sub_). | ||
| /// Defaults to <c>false</c>. | ||
| /// </param> | ||
| /// <returns>All symbols ordered by segment then offset.</returns> | ||
| IReadOnlyList<LstSymbol> GetAll(bool includeAutoLabels = false); |
| [McpServerTool(Name = "cryogenic_symbol_lookup", UseStructuredContent = true)] | ||
| [Description("Look up the DNCDPRG.lst symbol defined at a given segment and offset. Parameters: segment (string, e.g. \"seg000\"), offset (int, decimal value of the hex offset, e.g. 0x00B0 = 176). Returns found flag, address, and symbol details.")] | ||
| public SymbolLookupResponse CryogenicSymbolLookup(string segment, int offset) { | ||
| Logger.Information("MCP tool invoked: cryogenic_symbol_lookup Segment={Segment}, Offset=0x{OffsetHex:X4}", segment, offset); |
Comment on lines
+154
to
+176
| [McpServerTool(Name = "cryogenic_symbol_search", UseStructuredContent = true)] | ||
| [Description("Search the DNCDPRG.lst symbol table by name substring. Parameters: nameSubstring (string, case-insensitive, empty = all), includeAutoLabels (bool, default false). Returns list of matching symbols with segment, offset, and name.")] | ||
| public SymbolSearchResponse CryogenicSymbolSearch(string nameSubstring, bool includeAutoLabels) { | ||
| Logger.Information("MCP tool invoked: cryogenic_symbol_search Query={Query}, IncludeAutoLabels={IncludeAutoLabels}", nameSubstring, includeAutoLabels); | ||
| IReadOnlyList<LstSymbol> matches = _symbolTable.FindByName(nameSubstring, includeAutoLabels); | ||
| return new SymbolSearchResponse { | ||
| Query = nameSubstring, | ||
| IncludedAutoLabels = includeAutoLabels, | ||
| MatchCount = matches.Count, | ||
| Matches = matches.Select(ToSymbolEntry).ToList() | ||
| }; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Returns all named symbols from the DNCDPRG.lst symbol table, optionally including auto-labels. | ||
| /// </summary> | ||
| /// <param name="includeAutoLabels"> | ||
| /// When <c>true</c>, includes IDA-generated labels (loc_, sub_). | ||
| /// </param> | ||
| /// <returns>A <see cref="SymbolListResponse"/> containing all symbols in listing order.</returns> | ||
| [McpServerTool(Name = "cryogenic_symbols_list", UseStructuredContent = true)] | ||
| [Description("List all symbols from the embedded DNCDPRG.lst disassembly. Parameters: includeAutoLabels (bool, default false — omit loc_/sub_ labels). Returns the full symbol set with segment, hex offset, and name.")] | ||
| public SymbolListResponse CryogenicSymbolsList(bool includeAutoLabels) { |
Comment on lines
87
to
+100
| /// <summary> | ||
| /// Provides service instances available for MCP tool method injection. | ||
| /// No service instances are currently required for Cryogenic tools. | ||
| /// Supplies a singleton <see cref="ILstSymbolTable"/> built from the embedded DNCDPRG.lst | ||
| /// resource so <see cref="CryogenicSymbolMcpTools"/> can resolve it via constructor injection. | ||
| /// </summary> | ||
| /// <returns>Service instances for MCP tool execution.</returns> | ||
| public IEnumerable<object> GetMcpServices() { | ||
| Logger.Debug("Supplying MCP services collection (empty)."); | ||
| return Array.Empty<object>(); | ||
| ILstSymbolParser parser = new LstSymbolParser(); | ||
| LstSymbolTableFactory factory = new(parser); | ||
| ILstSymbolTable symbolTable = factory.Build(); | ||
| Logger.Information( | ||
| "Supplying ILstSymbolTable as MCP service. TotalSymbols={Total}", | ||
| symbolTable.GetAll(includeAutoLabels: true).Count); | ||
| return new object[] { symbolTable }; |
| ..\.github\workflows\release.yml = ..\.github\workflows\release.yml | ||
| EndProjectSection | ||
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Cryogenic.Tests", "Cryogenic.Tests\Cryogenic.Tests.csproj", "{116BEFB9-B935-45B6-83B3-ABE18B0E529F}" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Changes
src/Cryogenic/Symbols/with SRP classes:LstSymbol,LstSymbolLookup,ILstSymbolParser,ILstSymbolTable,LstSymbolParser,LstSymbolTable,LstSymbolTableFactory.ILstSymbolTable.FindByAddress(string segment, ushort offset)returns a non-nullableLstSymbolLookupwrapper record (Foundflag + non-nullSymbol). No Try pattern, no nullable returns, no exceptions in the symbol API.doc/DNCDPRG.lstasCryogenic.DNCDPRG.lst.CryogenicSymbolMcpToolsclass:cryogenic_symbol_lookup,cryogenic_symbol_search,cryogenic_symbols_list. The class receives itsILstSymbolTablethrough constructor injection — no static state, no calculated properties, no locks, no nullables.ILstSymbolTableis built once and registered as a singleton MCP service inDuneCdOverrideSupplier.GetMcpServices()so the ModelContextProtocol activator can resolveCryogenicSymbolMcpToolsvia DI._symbolTablefield, theSymbolTablecalculated property, and theSymbolTableLockfromCryogenicMcpTools.src/Cryogenic.Tests/xUnit project (net10.0) and added it toCryogenic.sln.AwesomeAssertions9.4.0 to the test project and structured all 30 tests with the Arrange / Act / Assert pattern usingShould()fluent assertions.// ═══banner comments introduced by this change.Rationale behind Changes
The IDA-style listing (
DNCDPRG.lst) carries 380 named symbols and 3303 auto-labels forseg000that are extremely useful for live reverse-engineering work. Exposing it through MCP tools lets the symbol names show up next to addresses during runtime inspection without manual lookups, and centralising parsing/lookup behind a small SRP-friendly API keeps the surface easy to test and extend. Splitting the symbol tools into their own non-static class with constructor-injected dependencies aligns with the project's convention of avoiding static state, calculated properties, locks for frozen data, and nullable types. Returning aLstSymbolLookupwrapper instead of aTry-style API or a nullable result keeps the public contract free of nullables and exceptions while still letting callers branch on a simpleFoundflag. The test refactor follows the project's preferred testing convention (AAA + AwesomeAssertions).Suggested Testing Steps
src/, rundotnet build— should be 0 warnings, 0 errors.src/, rundotnet test Cryogenic.Tests/Cryogenic.Tests.csproj— all 30 tests should pass.cryogenic_symbol_lookupforseg000:0000(expectFound = truewithstart),cryogenic_symbol_searchforplay_intro, andcryogenic_symbols_list(expect a non-empty named-symbol list).