fix(skill): preserve skillContent/configJson on security scan write-back (issue #45)

SkillPackageResolver.persistScanOutcome built a fresh SkillEntity with only
id + scan fields, then called updateById. SkillEntity declares six columns
with @TableField(updateStrategy = FieldStrategy.ALWAYS) — name_zh, name_en,
config_json, source_code, skill_content, security_scan_result — so the
ALWAYS strategy emits UPDATE statements that write NULL to every one of
those columns not set on the partial entity.

Effect: every security re-scan that produced a status/findings change
silently wiped skill_content, config_json, source_code, name_zh, name_en
on the row. After importing a custom skill, the first scan tick destroyed
the imported content.

Fix: switch to LambdaUpdateWrapper so the UPDATE only touches the three
scan columns we actually want to change. Other skillMapper.updateById
call sites (SkillService, BuiltinSkillSeedService) pass DB-hydrated
existing entities and are unaffected.

Reported and diagnosed by @pipima9950-glitch in issue #45.
This commit is contained in:
matevip 2026-04-30 15:28:56 +08:00
parent 977e181949
commit d20b440ce5

View File

@ -1,5 +1,6 @@
package vip.mate.skill.runtime;
import com.baomidou.mybatisplus.core.conditions.update.LambdaUpdateWrapper;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
@ -106,17 +107,24 @@ public class SkillPackageResolver {
}
try {
SkillEntity update = new SkillEntity();
update.setId(entity.getId());
update.setSecurityScanStatus(newStatus);
update.setSecurityScanResult(newJson);
update.setSecurityScanTime(LocalDateTime.now());
skillMapper.updateById(update);
// Whitelist via LambdaUpdateWrapper (issue #45): SkillEntity has
// several @TableField(updateStrategy = FieldStrategy.ALWAYS)
// columns (skill_content, config_json, source_code, name_zh,
// name_en, security_scan_result). Calling updateById with a
// partial entity would tell MyBatis Plus to write NULL to every
// ALWAYS column not set on the partial wiping the imported
// skill content on every scan write-back.
LocalDateTime now = LocalDateTime.now();
skillMapper.update(null, new LambdaUpdateWrapper<SkillEntity>()
.eq(SkillEntity::getId, entity.getId())
.set(SkillEntity::getSecurityScanStatus, newStatus)
.set(SkillEntity::getSecurityScanResult, newJson)
.set(SkillEntity::getSecurityScanTime, now));
// Keep the in-memory entity coherent with the DB so the next
// resolve in the same tick doesn't redundantly write again.
entity.setSecurityScanStatus(newStatus);
entity.setSecurityScanResult(newJson);
entity.setSecurityScanTime(update.getSecurityScanTime());
entity.setSecurityScanTime(now);
} catch (Exception e) {
log.warn("Failed to persist scan outcome for skill '{}': {}", entity.getName(), e.getMessage());
}