fix(skill): encode bridge virtual ids in the top two bits

This commit is contained in:
matevip 2026-05-14 18:18:21 +08:00
parent 139a478bcd
commit 1bba59fc7e
3 changed files with 108 additions and 28 deletions

View File

@ -58,23 +58,36 @@ import java.util.concurrent.ConcurrentHashMap;
* page always shows current state.</li>
* </ul>
*
* <p>ID namespace: virtual skill ids use a high sentinel
* {@link #VIRTUAL_ID_BASE} different from the MCP bridge's, so the two
* id spaces never collide and a callsite can dispatch on which bridge
* owns an id without coordination.
* <p>ID namespace: virtual ACP ids set both top bits of a {@code long}
* (bit 63 + bit 62) so they sit in a different type-tag than MCP
* (which sets only bit 63 see
* {@link vip.mate.skill.mcp.McpSkillBridge}). The bottom 62 bits carry
* the underlying endpointId. The earlier {@code 8e18 + endpointId}
* addition scheme broke once Snowflake-issued endpoint ids crossed
* the {@code 1e17} bound, so the bit-tagged layout replaces it.
*
* <p>{@code VIRTUAL_ID_BASE + smallId} still equals
* {@code VIRTUAL_ID_BASE | smallId} for any {@code smallId < 2^62}, so
* test fixtures that build virtual ids by addition continue to work.
*/
@Slf4j
@Service
public class AcpSkillBridge {
/** Type tag for ACP virtual ids: bits 63 + 62 set. */
public static final long VIRTUAL_ID_BASE = 0xC000000000000000L;
/**
* High sentinel for ACP virtual id space. Distinct from
* {@code McpSkillBridge.VIRTUAL_ID_BASE} (9e18) so the two virtual
* spaces are partitionable by simple range checks.
* @deprecated The bound is implicit in the bit-tag layout any id
* whose top two bits are both set is an ACP virtual id. Kept
* for source compatibility with earlier callers.
*/
public static final long VIRTUAL_ID_BASE = 8_000_000_000_000_000_000L;
/** Upper bound, exclusive — anything in [BASE, BASE + 1e17) is ours. */
public static final long VIRTUAL_ID_BOUND = VIRTUAL_ID_BASE + 100_000_000_000_000_000L;
@Deprecated
public static final long VIRTUAL_ID_BOUND = -1L; // 0xFFFFFFFFFFFFFFFFL
/** Selects the top-two type-tag bits. */
private static final long TAG_MASK = 0xC000000000000000L;
/** Selects the bottom 62 bits that carry the original endpoint id. */
private static final long ID_MASK = 0x3FFFFFFFFFFFFFFFL;
private final AcpEndpointService endpointService;
private final AcpDelegationService delegationService;
@ -100,16 +113,22 @@ public class AcpSkillBridge {
}
public static boolean isVirtualAcpSkillId(Long id) {
return id != null && id >= VIRTUAL_ID_BASE && id < VIRTUAL_ID_BOUND;
return id != null && (id & TAG_MASK) == VIRTUAL_ID_BASE;
}
public static Long extractEndpointId(Long virtualId) {
if (!isVirtualAcpSkillId(virtualId)) return null;
return virtualId - VIRTUAL_ID_BASE;
return virtualId & ID_MASK;
}
public static long virtualIdFor(AcpEndpointEntity endpoint) {
return VIRTUAL_ID_BASE + endpoint.getId();
long eid = endpoint.getId();
if ((eid & TAG_MASK) != 0L) {
throw new IllegalStateException(
"ACP endpoint id 0x" + Long.toHexString(eid)
+ " uses the top two bits — would collide with the virtual id type tag");
}
return VIRTUAL_ID_BASE | eid;
}
@PostConstruct

View File

@ -44,11 +44,27 @@ import java.util.Map;
* links back to the MCP page).</li>
* </ul>
*
* <p>ID namespace: virtual skill ids use a high sentinel
* {@link #VIRTUAL_ID_BASE} + mcpServerId so they can never collide
* with real {@code mate_skill.id} values (Snowflake longs are bounded
* well below this base). Negative numbers were considered but several
* existing endpoints {@code abs()} the id for path constraints.
* <p>ID namespace: virtual ids encode a 2-bit type tag in the top two
* bits of a {@code long}, leaving 62 bits to carry the underlying
* mcpServerId:
* <pre>
* bit 63 (sign) | bit 62 | bits 0..61
* --------------+--------+--------------------------------
* 0 | 0 | real persisted skill (Snowflake)
* 1 | 0 | virtual MCP-derived skill
* 1 | 1 | virtual ACP-derived skill
* </pre>
*
* <p>The earlier {@code 9e18 + serverId} addition scheme broke once
* Snowflake-issued mcpServerIds crossed ~{@code 2e18} (the sum then
* overflowed signed long, wrapping to a negative number that no longer
* satisfied {@code id >= 9e18} every detail / lookup of a freshly
* created MCP server 500'd with "技能不存在"). The bit-tagged layout
* has no arithmetic and survives any 62-bit server id.
*
* <p>The constants are arranged so that {@code BASE + smallId} still
* equals {@code BASE | smallId} for any {@code smallId < 2^62}, so
* test fixtures that build virtual ids by addition keep working.
*/
@Slf4j
@Service
@ -56,33 +72,43 @@ import java.util.Map;
public class McpSkillBridge {
/**
* High sentinel for virtual id space. Snowflake ids fit in 63 bits
* but in practice never approach this magnitude, so anything
* {@code >= VIRTUAL_ID_BASE} is unambiguously a bridged MCP skill.
* Type tag for the MCP virtual id space: bit 63 set, bit 62 clear.
* Equal to {@link Long#MIN_VALUE}; named for the historical
* "base sentinel" idiom callers still use.
*/
public static final long VIRTUAL_ID_BASE = 9_000_000_000_000_000_000L;
public static final long VIRTUAL_ID_BASE = Long.MIN_VALUE; // 0x8000000000000000L
/** Selects the top-two type-tag bits. */
private static final long TAG_MASK = 0xC000000000000000L;
/** Selects the bottom 62 bits that carry the original server id. */
private static final long ID_MASK = 0x3FFFFFFFFFFFFFFFL;
private final McpServerService mcpServerService;
private final McpClientManager mcpClientManager;
private final ObjectMapper objectMapper;
/**
* @return true iff the given id falls inside the virtual MCP skill
* range. Cheap O(1) check, callers use it to route lookups
* between the real DB and this bridge.
* @return true iff the given id carries the MCP virtual-skill type
* tag (bit 63 set, bit 62 clear). Cheap O(1) bit-mask check.
*/
public static boolean isVirtualMcpSkillId(Long id) {
return id != null && id >= VIRTUAL_ID_BASE;
return id != null && (id & TAG_MASK) == VIRTUAL_ID_BASE;
}
/** Inverse mapping: extract the original MCP server id. */
public static Long extractMcpServerId(Long virtualId) {
if (!isVirtualMcpSkillId(virtualId)) return null;
return virtualId - VIRTUAL_ID_BASE;
return virtualId & ID_MASK;
}
public static long virtualIdFor(McpServerEntity server) {
return VIRTUAL_ID_BASE + server.getId();
long sid = server.getId();
if ((sid & TAG_MASK) != 0L) {
throw new IllegalStateException(
"MCP server id 0x" + Long.toHexString(sid)
+ " uses the top two bits — would collide with the virtual id type tag");
}
return VIRTUAL_ID_BASE | sid;
}
/**

View File

@ -108,6 +108,41 @@ class McpSkillBridgeManifestTest {
"no prefixed tool name expected, got: " + entity.getManifestJson());
}
@Test
@DisplayName("virtual id encoding round-trips for Snowflake-magnitude server ids (regression)")
void virtualIdRoundTripsForLargeSnowflakeIds() {
// 2054864660577071106 is a real Snowflake observed in the wild;
// the previous BASE + serverId scheme overflowed signed long for
// ids of this magnitude, producing negative virtual ids that
// failed isVirtualMcpSkillId and broke the skill detail lookup.
long[] cases = {1L, 1_000_001L, 2_054_864_660_577_071_106L, (1L << 61), (1L << 62) - 1L};
for (long sid : cases) {
McpServerEntity server = newServer(sid, "anything");
long vid = McpSkillBridge.virtualIdFor(server);
assertTrue(McpSkillBridge.isVirtualMcpSkillId(vid),
"vid for serverId=" + sid + " should be classified as MCP virtual: got 0x"
+ Long.toHexString(vid));
assertEquals(sid, McpSkillBridge.extractMcpServerId(vid),
"extract did not round-trip for serverId=" + sid);
}
}
@Test
@DisplayName("MCP and ACP virtual id spaces never overlap, real Snowflake ids classify as neither")
void virtualIdSpacesAreDisjoint() {
long serverId = 2_054_864_660_577_071_106L; // Snowflake magnitude
McpServerEntity server = newServer(serverId, "anything");
long mcpVid = McpSkillBridge.virtualIdFor(server);
assertTrue(McpSkillBridge.isVirtualMcpSkillId(mcpVid));
// An MCP virtual id must NOT be misread as ACP.
assertTrue(!vip.mate.skill.acp.AcpSkillBridge.isVirtualAcpSkillId(mcpVid),
"MCP vid 0x" + Long.toHexString(mcpVid) + " leaked into the ACP range");
// Real Snowflake ids (positive, top bits clear) must be neither.
assertTrue(!McpSkillBridge.isVirtualMcpSkillId(serverId));
assertTrue(!vip.mate.skill.acp.AcpSkillBridge.isVirtualAcpSkillId(serverId));
}
@Test
@DisplayName("CJK-only server names slug to a stable id-based fallback instead of an all-dash collision")
void cjkOnlyNameFallsBackToIdSlug() {